Merge branch '26-fix-all-clippy-advice' into 'main'

Resolve "Fix all clippy advice"

Closes #26

See merge request guillaume54/barb!6

5 files changed, 98 insertions, 100 deletions

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 019530a..c34e8f3 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -13,10 +13,14 @@ test:
   before_script:
     - apt update && apt install libjq1 libjq-dev libonig-dev libonig5
     - rustup component add rustfmt
+    - rustup component add clippy
   script:
     - cargo fmt -- --check
-    - cargo build --tests
-    - cargo test --all
+    - cargo build --all-features --tests
+    - cargo clippy
+    - cargo clippy --features jq
+    - cargo clippy --tests
+    - cargo test --all-features
 
 # pretty:
 #   stage: test
diff --git a/src/barbfile.rs b/src/barbfile.rs
index e5ef916..884dcee 100644
--- a/src/barbfile.rs
+++ b/src/barbfile.rs
@@ -8,9 +8,6 @@ use std::str::FromStr;
 use std::string::ToString;
 use std::{error::Error, fmt};
 
-#[cfg(feature = "jq")]
-use jq_rs;
-
 #[derive(Debug)]
 pub struct BarbParseError {}
 
@@ -27,11 +24,11 @@ impl fmt::Display for BarbParseError {
 }
 
 pub enum Method {
-    GET,
-    PUT,
-    POST,
-    PATCH,
-    DELETE,
+    Get,
+    Put,
+    Post,
+    Patch,
+    Delete,
 }
 
 impl FromStr for Method {
@@ -39,11 +36,11 @@ impl FromStr for Method {
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         match s {
-            "GET" => Ok(Self::GET),
-            "PUT" => Ok(Self::PUT),
-            "POST" => Ok(Self::POST),
-            "PATCH" => Ok(Self::PATCH),
-            "DELETE" => Ok(Self::DELETE),
+            "GET" => Ok(Self::Get),
+            "PUT" => Ok(Self::Put),
+            "POST" => Ok(Self::Post),
+            "PATCH" => Ok(Self::Patch),
+            "DELETE" => Ok(Self::Delete),
             _ => Err(BarbParseError {}),
         }
     }
@@ -52,22 +49,18 @@ impl FromStr for Method {
 impl ToString for Method {
     fn to_string(&self) -> String {
         match self {
-            Self::GET => String::from("GET"),
-            Self::PUT => String::from("PUT"),
-            Self::POST => String::from("POST"),
-            Self::PATCH => String::from("PATCH"),
-            Self::DELETE => String::from("DELETE"),
+            Self::Get => String::from("GET"),
+            Self::Put => String::from("PUT"),
+            Self::Post => String::from("POST"),
+            Self::Patch => String::from("PATCH"),
+            Self::Delete => String::from("DELETE"),
         }
     }
 }
 
 impl Method {
     pub fn takes_body(&self) -> bool {
-        match self {
-            Method::GET => false,
-            Method::DELETE => false,
-            _ => true,
-        }
+        !matches!(self, Method::Get | Method::Delete)
     }
 }
 
@@ -94,8 +87,8 @@ impl Header {
 }
 
 enum FilterType {
-    JQ,
-    PATH,
+    Jq,
+    Path,
 }
 
 pub struct BarbFilter {
@@ -118,8 +111,8 @@ impl FromStr for BarbFilter {
             },
             String::from(&groups["filter"]),
             match &groups["type"] {
-                "|" => FilterType::JQ,
-                _ => FilterType::PATH,
+                "|" => FilterType::Jq,
+                _ => FilterType::Path,
             },
         ))
     }
@@ -144,7 +137,7 @@ impl BarbFilter {
     pub fn from_path(filter: String) -> BarbFilter {
         BarbFilter {
             name: None,
-            filter_type: FilterType::PATH,
+            filter_type: FilterType::Path,
             filter,
         }
     }
@@ -159,23 +152,23 @@ impl BarbFilter {
     }
 
     #[cfg(feature = "jq")]
-    fn apply_jq(&self, body: &String) -> Result<String, String> {
-        if let FilterType::JQ = self.filter_type {
+    fn apply_jq(&self, body: &str) -> Result<String, String> {
+        if let FilterType::Path = self.filter_type {
             return Err(String::from("Incorrect filter type"));
         }
 
-        jq_rs::run(self.filter.as_str(), body.as_str())
+        jq_rs::run(self.filter.as_str(), body)
             .map_err(|x| x.to_string())
             .map(|x| String::from(x.trim().trim_matches('"')))
     }
 
-    fn apply_path(&self, body: &String) -> Result<String, String> {
-        if let FilterType::JQ = self.filter_type {
+    fn apply_path(&self, body: &str) -> Result<String, String> {
+        if let FilterType::Jq = self.filter_type {
             return Err(String::from("Incorrect filter type"));
         }
 
-        let json: Value = serde_json::from_str(body.as_str())
-            .map_err(|_| String::from("Failed to decode body"))?;
+        let json: Value =
+            serde_json::from_str(body).map_err(|_| String::from("Failed to decode body"))?;
         let path = &json.path(self.filter.as_str())?;
         Ok(match path {
             Value::Array(val) => match val.len() {
@@ -190,11 +183,12 @@ impl BarbFilter {
         .to_string())
     }
 
-    pub fn apply(&self, body: &String) -> Result<String, String> {
+    pub fn apply(&self, body: &str) -> Result<String, String> {
         match self.filter_type {
             #[cfg(feature = "jq")]
-            FilterType::JQ => self.apply_jq(body),
-            FilterType::PATH => self.apply_path(body),
+            FilterType::Jq => self.apply_jq(body),
+            FilterType::Path => self.apply_path(body),
+            #[cfg(not(feature = "jq"))]
             _ => Ok(body.to_string()),
         }
     }
@@ -264,7 +258,7 @@ impl BarbFile {
         if !dep_path.is_absolute() {
             let my_path = Path::new(&self.file_name)
                 .parent()
-                .or(Some(Path::new("")))?;
+                .or_else(|| Some(Path::new("")))?;
             return Some(String::from(my_path.join(dep_path).to_str()?));
         }
 
@@ -296,7 +290,7 @@ fn decode_url_line(line: &str) -> Result<(Method, String), BarbParseError> {
     let mut components = line[1..].split('^');
     let meth = components.next().ok_or(BarbParseError {})?;
     let url = components.next().ok_or(BarbParseError {})?;
-    return Ok((Method::from_str(meth)?, String::from(url)));
+    Ok((Method::from_str(meth)?, String::from(url)))
 }
 
 fn decode_header(line: &str) -> Result<Header, BarbParseError> {
@@ -333,24 +327,23 @@ impl FromStr for BarbFile {
         let mut dependency: Option<String> = None;
 
         for line in &mut lines {
-            if line == "" {
+            if line.is_empty() {
                 // End of header.
                 break;
             }
 
-            if let Some(_) = line.find(':') {
+            if line.find(':').is_some() {
                 headers.push(decode_header(line).map_err(|_| BarbParseError {})?);
             }
 
             if let Some('>') = line.chars().nth(1) {
-                dependency = line.get(2..).map(|x| String::from(x));
+                dependency = line.get(2..).map(String::from);
             }
 
             if BarbFilter::is_match(String::from(line)) {
-                match BarbFilter::from_str(line) {
-                    Ok(filter) => filters.push(filter),
-                    Err(_) => (),
-                };
+                if let Ok(filter) = BarbFilter::from_str(line) {
+                    filters.push(filter);
+                }
             }
         }
 
@@ -359,7 +352,7 @@ impl FromStr for BarbFile {
         Ok(BarbFile {
             file_name: String::from(""),
             preamble: BarbPreamble::new(method, url, headers, filters, dependency),
-            body: if body == "" { None } else { Some(body) },
+            body: if body.is_empty() { None } else { Some(body) },
         })
     }
 }
@@ -370,29 +363,29 @@ mod tests {
 
     #[test]
     fn test_method_from_str() {
-        assert!(matches!(Method::from_str("GET").unwrap(), Method::GET));
-        assert!(matches!(Method::from_str("PUT").unwrap(), Method::PUT));
-        assert!(matches!(Method::from_str("POST").unwrap(), Method::POST));
-        assert!(matches!(Method::from_str("PATCH").unwrap(), Method::PATCH));
+        assert!(matches!(Method::from_str("GET").unwrap(), Method::Get));
+        assert!(matches!(Method::from_str("PUT").unwrap(), Method::Put));
+        assert!(matches!(Method::from_str("POST").unwrap(), Method::Post));
+        assert!(matches!(Method::from_str("PATCH").unwrap(), Method::Patch));
         assert!(matches!(
             Method::from_str("DELETE").unwrap(),
-            Method::DELETE
+            Method::Delete
         ));
     }
 
     #[test]
     fn test_method_takes_body() {
-        assert!(!Method::GET.takes_body());
-        assert!(Method::PUT.takes_body());
-        assert!(Method::POST.takes_body());
-        assert!(Method::PATCH.takes_body());
-        assert!(!Method::DELETE.takes_body());
+        assert!(!Method::Get.takes_body());
+        assert!(Method::Put.takes_body());
+        assert!(Method::Post.takes_body());
+        assert!(Method::Patch.takes_body());
+        assert!(!Method::Delete.takes_body());
     }
 
     #[test]
     fn test_decode_url_line() {
         let (method, url) = decode_url_line("#GET^http://blahblah").unwrap();
-        assert!(matches!(method, Method::GET));
+        assert!(matches!(method, Method::Get));
         assert_eq!(url, "http://blahblah");
     }
 
@@ -408,7 +401,7 @@ mod tests {
         let barbfile =
             BarbFile::from_str("#GET^https://blah.com/api/blah\n#Authorization: BLAH\n#|filtr\n")
                 .unwrap();
-        assert!(matches!(barbfile.preamble.method, Method::GET));
+        assert!(matches!(barbfile.preamble.method, Method::Get));
         assert_eq!(barbfile.preamble.url, "https://blah.com/api/blah");
         assert_eq!(
             barbfile.preamble.filters[0].filter(),
@@ -425,7 +418,7 @@ mod tests {
         let barbfile =
             BarbFile::from_str("#POST^https://blah.com/api/blah\n#Authorization: BLAH\n#|filtr\n\n{\"key\":\"value\"}\n")
                 .unwrap();
-        assert!(matches!(barbfile.preamble.method, Method::POST));
+        assert!(matches!(barbfile.preamble.method, Method::Post));
         assert_eq!(barbfile.preamble.url, "https://blah.com/api/blah");
         assert_eq!(
             barbfile.preamble.filters[0].filter(),
@@ -442,7 +435,7 @@ mod tests {
         let filter = BarbFilter::from_str("#FOO|.bar.foo").unwrap();
         assert_eq!(filter.name, Some(String::from("FOO")));
         assert_eq!(filter.filter, String::from(".bar.foo"));
-        assert!(matches!(filter.filter_type, FilterType::JQ));
+        assert!(matches!(filter.filter_type, FilterType::Jq));
     }
 
     #[test]
@@ -450,7 +443,7 @@ mod tests {
         let filter = BarbFilter::from_str("#|.bar.foo").unwrap();
         assert_eq!(filter.name, None);
         assert_eq!(filter.filter, String::from(".bar.foo"));
-        assert!(matches!(filter.filter_type, FilterType::JQ));
+        assert!(matches!(filter.filter_type, FilterType::Jq));
     }
 
     #[test]
@@ -458,7 +451,7 @@ mod tests {
         let filter = BarbFilter::from_str("#FOO$$.bar.foo").unwrap();
         assert_eq!(filter.name, Some(String::from("FOO")));
         assert_eq!(filter.filter, String::from("$.bar.foo"));
-        assert!(matches!(filter.filter_type, FilterType::PATH));
+        assert!(matches!(filter.filter_type, FilterType::Path));
     }
 
     #[test]
@@ -466,7 +459,7 @@ mod tests {
         let filter = BarbFilter::from_str("#$$.bar.foo").unwrap();
         assert_eq!(filter.name, None);
         assert_eq!(filter.filter, String::from("$.bar.foo"));
-        assert!(matches!(filter.filter_type, FilterType::PATH));
+        assert!(matches!(filter.filter_type, FilterType::Path));
     }
 
     #[cfg(feature = "jq")]
diff --git a/src/executor.rs b/src/executor.rs
index 6b1bc5a..b296c65 100644
--- a/src/executor.rs
+++ b/src/executor.rs
@@ -2,7 +2,6 @@ use crate::barbfile::{BarbFile, BarbFilter};
 use crate::output::BarbOutput;
 use regex::Regex;
 use std::collections::HashMap;
-use ureq;
 use ureq::Error as UreqError;
 
 pub struct Context {
@@ -28,21 +27,21 @@ impl Context {
     }
 
     #[cfg(test)]
-    fn key_str(&self, key: &String) -> String {
-        // Use for reference / utility?
+    fn key_str(&self, key: &str) -> String {
         format!("{{{}}}", key)
     }
 
-    pub fn substitute(&self, string: &String) -> String {
-        let mut buffer = string.clone();
+    pub fn substitute(&self, string: &str) -> String {
+        let mut buffer = string.to_string();
         let re = Regex::new(r"\{(?P<key>[A-Za-z0-9_]+)(?::-(?P<value>.*))?\}").unwrap();
+        let empty_string = String::from("");
 
         for var in re.captures_iter(string) {
             let key = String::from(&var["key"]);
             let value = String::from(
                 var.name("value")
                     .map(|m| m.as_str())
-                    .or(Some(&String::from("")))
+                    .or(Some(&empty_string))
                     .unwrap(),
             );
 
@@ -75,7 +74,7 @@ impl Executor {
     ) -> ureq::Request {
         let mut req = ureq::request(
             bfile.method_as_string().as_str(),
-            self.context.substitute(&bfile.url()).as_str(),
+            self.context.substitute(bfile.url()).as_str(),
         );
 
         let mut final_headers: HashMap<String, String> = HashMap::new();
@@ -104,8 +103,8 @@ impl Executor {
         arg_filter: &Option<String>,
     ) -> Result<String, String> {
         if let Some(filter) = arg_filter {
-            return Ok(BarbFilter::from_path(filter.to_string()).apply(&body)?);
-        } else if bfile.filters().len() > 0 {
+            return BarbFilter::from_path(filter.to_string()).apply(&body);
+        } else if !bfile.filters().is_empty() {
             let mut end_body: String = body.clone();
             for filter in bfile.filters().iter() {
                 if let Some(name) = filter.name() {
@@ -116,7 +115,7 @@ impl Executor {
             }
             return Ok(end_body);
         }
-        Ok(String::from(body))
+        Ok(body)
     }
 
     fn run(&self, bfile: &BarbFile, req: ureq::Request) -> Result<ureq::Response, String> {
@@ -131,20 +130,20 @@ impl Executor {
         match resp {
             Ok(resp) => Ok(resp),
             Err(UreqError::Status(_, resp)) => Ok(resp),
-            Err(UreqError::Transport(transp)) => Err(String::from(transp.to_string())),
+            Err(UreqError::Transport(transp)) => Err(transp.to_string()),
         }
     }
 
     pub fn execute_dep(&mut self, bfile: &BarbFile, output: &BarbOutput) -> Result<(), String> {
-        let req = self.make_req(&bfile, vec![], output);
+        let req = self.make_req(bfile, vec![], output);
         let method = String::from(req.method());
         let url = String::from(req.url());
 
-        let response = self.run(&bfile, req)?;
+        let response = self.run(bfile, req)?;
 
         output.req_dep(method, url, response.status(), response.status_text());
         output.end_req();
-        self.apply_filters(&bfile, response.into_string().unwrap(), &None)?;
+        self.apply_filters(bfile, response.into_string().unwrap(), &None)?;
 
         Ok(())
     }
@@ -157,12 +156,12 @@ impl Executor {
         skip_filters: bool,
         headers: Vec<(String, String)>,
     ) -> Result<(), String> {
-        let req = self.make_req(&bfile, headers, output);
+        let req = self.make_req(bfile, headers, output);
 
         output.req(String::from(req.method()), String::from(req.url()));
         output.end_req();
 
-        let response = self.run(&bfile, req)?;
+        let response = self.run(bfile, req)?;
 
         output.status(response.status(), response.status_text());
         for header_name in response.headers_names() {
@@ -174,7 +173,7 @@ impl Executor {
         output.end_resp_hdr();
 
         if !skip_filters {
-            output.body(self.apply_filters(&bfile, response.into_string().unwrap(), filter)?);
+            output.body(self.apply_filters(bfile, response.into_string().unwrap(), filter)?);
         } else {
             output.body(response.into_string().unwrap());
         }
diff --git a/src/main.rs b/src/main.rs
index 6ebfc45..df12e1e 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -68,9 +68,9 @@ impl Args {
     }
 }
 
-fn read_file_barb(file_name: &String) -> Result<BarbFile, String> {
+fn read_file_barb(file_name: &str) -> Result<BarbFile, String> {
     BarbFile::from_file(file_name.to_string())
-        .map_err(|_| String::from(format!("Failed to parse file {}", file_name)))
+        .map_err(|_| format!("Failed to parse file {}", file_name))
 }
 
 fn main() {
@@ -79,18 +79,20 @@ fn main() {
     let mut executor = Executor::new(Context::new(env::vars()));
     let output = args.output();
 
-    let files: Vec<Result<BarbFile, String>> = args.files_iter().map(read_file_barb).collect();
+    let files: Vec<Result<BarbFile, String>> =
+        args.files_iter().map(|x| read_file_barb(x)).collect();
 
-    let (maybe_deps, errors): (Vec<Result<BarbFile, String>>, Vec<Result<BarbFile, String>>) =
-        files
-            .iter()
-            .map(|x| match x.as_ref().ok() {
-                Some(bfile) => bfile.dependency(),
-                None => None,
-            })
-            .filter(|x| x.is_some())
-            .map(|x| read_file_barb(&String::from(x.unwrap())))
-            .partition(|x| x.is_ok());
+    type MaybeBarbFiles = Vec<Result<BarbFile, String>>;
+
+    let (maybe_deps, errors): (MaybeBarbFiles, MaybeBarbFiles) = files
+        .iter()
+        .map(|x| match x.as_ref().ok() {
+            Some(bfile) => bfile.dependency(),
+            None => None,
+        })
+        .filter(|x| x.is_some())
+        .map(|x| read_file_barb(&x.unwrap()))
+        .partition(|x| x.is_ok());
 
     for e in errors {
         println!("{}", e.err().unwrap());
@@ -104,7 +106,7 @@ fn main() {
     dependencies.dedup();
 
     for dep in dependencies {
-        match executor.execute_dep(&dep, &output) {
+        match executor.execute_dep(dep, &output) {
             Ok(()) => (),
             Err(err) => println!("{}", err),
         }
diff --git a/src/output.rs b/src/output.rs
index 0f0e4a9..ebc3fec 100644
--- a/src/output.rs
+++ b/src/output.rs
@@ -102,7 +102,7 @@ impl BarbOutput {
 
     pub fn end_req(&self) {
         if self.req_headers || self.request {
-            println!("");
+            println!();
         }
     }
 
@@ -119,7 +119,7 @@ impl BarbOutput {
 
     pub fn end_resp_hdr(&self) {
         if self.headers {
-            println!("");
+            println!();
         }
     }