From f40f73dc8e26e6d8730344f4c6cbb47ee3d48342 Mon Sep 17 00:00:00 2001
From: Matan Kushner <hello@matchai.me>
Date: Wed, 31 Jul 2019 19:48:51 -0400
Subject: [PATCH] fix: Address longstanding linter errors

---
 ci/cargo-clippy.yml       |  2 +-
 src/config.rs             | 36 +++++++++++++++---------------------
 src/context.rs            | 20 +++++++++-----------
 src/init.rs               |  3 +--
 src/module.rs             | 32 ++++++++++++++++----------------
 src/modules/directory.rs  |  8 ++++----
 src/modules/git_branch.rs |  4 ++--
 src/modules/git_status.rs |  8 ++++----
 src/modules/golang.rs     |  6 +++---
 src/modules/package.rs    |  4 ++--
 src/modules/python.rs     |  6 +++---
 src/modules/username.rs   |  4 ++--
 src/segment.rs            |  8 ++++----
 13 files changed, 66 insertions(+), 75 deletions(-)

diff --git a/ci/cargo-clippy.yml b/ci/cargo-clippy.yml
index 76680ceb..154acfe2 100644
--- a/ci/cargo-clippy.yml
+++ b/ci/cargo-clippy.yml
@@ -9,5 +9,5 @@ jobs:
     steps:
       - template: install-rust.yml
 
-      - script: cargo clippy -- -D warnings -D clippy::all -D clippy::nursery -D clippy::pedantic
+      - script: cargo clippy -- -D clippy::all
         displayName: Run clippy
diff --git a/src/config.rs b/src/config.rs
index 92073b5f..ba339468 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -21,30 +21,27 @@ pub trait Config {
 impl Config for Table {
     /// Initialize the Config struct
     fn initialize() -> Table {
-        if let Some(file_data) = Table::config_from_file() {
+        if let Some(file_data) = Self::config_from_file() {
             return file_data;
         }
 
-        Table::new()
+        Self::new()
     }
 
     /// Create a config from a starship configuration file
     fn config_from_file() -> Option<Table> {
-        let file_path = match env::var("STARSHIP_CONFIG") {
-            Ok(path) => {
-                // Use $STARSHIP_CONFIG as the config path if available
-                log::debug!("STARSHIP_CONFIG is set: \n{}", &path);
-                path
-            }
-            Err(_) => {
-                // Default to using ~/.config/starhip.toml
-                log::debug!("STARSHIP_CONFIG is not set");
-                let config_path = home_dir()?.join(".config/starship.toml");
-                let config_path_str = config_path.to_str()?.to_owned();
+        let file_path = if let Ok(path) = env::var("STARSHIP_CONFIG") {
+            // Use $STARSHIP_CONFIG as the config path if available
+            log::debug!("STARSHIP_CONFIG is set: \n{}", &path);
+            path
+        } else {
+            // Default to using ~/.config/starhip.toml
+            log::debug!("STARSHIP_CONFIG is not set");
+            let config_path = home_dir()?.join(".config/starship.toml");
+            let config_path_str = config_path.to_str()?.to_owned();
 
-                log::debug!("Using default config path: {}", config_path_str);
-                config_path_str
-            }
+            log::debug!("Using default config path: {}", config_path_str);
+            config_path_str
         };
 
         let toml_content = match utils::read_file(&file_path) {
@@ -65,10 +62,7 @@ impl Config for Table {
 
     /// Get the subset of the table for a module by its name
     fn get_module_config(&self, module_name: &str) -> Option<&toml::value::Table> {
-        let module_config = self
-            .get(module_name)
-            .map(toml::Value::as_table)
-            .unwrap_or(None);
+        let module_config = self.get(module_name).and_then(toml::Value::as_table);
 
         if module_config.is_some() {
             log::debug!(
@@ -149,7 +143,7 @@ impl Config for Table {
     }
 }
 
-
+#[cfg(test)]
 mod tests {
     use super::*;
 
diff --git a/src/context.rs b/src/context.rs
index 3e3640ac..1cfbaa3a 100644
--- a/src/context.rs
+++ b/src/context.rs
@@ -70,10 +70,10 @@ impl<'a> Context<'a> {
         let repository = Repository::discover(&current_dir).ok();
         let repo_root = repository
             .as_ref()
-            .and_then(|repo| repo.workdir().map(|repo| repo.to_path_buf()));
+            .and_then(|repo| repo.workdir().map(std::path::Path::to_path_buf));
         let branch_name = repository
             .as_ref()
-            .and_then(|repo| get_current_branch(&repo));
+            .and_then(|repo| get_current_branch(repo));
 
         Context {
             config,
@@ -102,9 +102,7 @@ impl<'a> Context<'a> {
         let config = self.config.get_module_config(name);
 
         // If the segment has "disabled" set to "true", don't show it
-        let disabled = config
-            .map(|table| table.get_as_bool("disabled"))
-            .unwrap_or(None);
+        let disabled = config.and_then(|table| table.get_as_bool("disabled"));
 
         if disabled == Some(true) {
             return None;
@@ -135,17 +133,17 @@ pub struct ScanDir<'a> {
 }
 
 impl<'a> ScanDir<'a> {
-    pub fn set_files(mut self, files: &'a [&'a str]) -> Self {
+    pub const fn set_files(mut self, files: &'a [&'a str]) -> Self {
         self.files = files;
         self
     }
 
-    pub fn set_extensions(mut self, extensions: &'a [&'a str]) -> Self {
+    pub const fn set_extensions(mut self, extensions: &'a [&'a str]) -> Self {
         self.extensions = extensions;
         self
     }
 
-    pub fn set_folders(mut self, folders: &'a [&'a str]) -> Self {
+    pub const fn set_folders(mut self, folders: &'a [&'a str]) -> Self {
         self.folders = folders;
         self
     }
@@ -155,9 +153,9 @@ impl<'a> ScanDir<'a> {
     pub fn scan(&mut self) -> bool {
         self.dir_files.iter().any(|path| {
             if path.is_dir() {
-                return path_has_name(&path, &self.folders);
+                path_has_name(path, self.folders)
             } else {
-                return path_has_name(&path, &self.files) || has_extension(&path, &self.extensions);
+                path_has_name(path, self.files) || has_extension(path, self.extensions)
             }
         })
     }
@@ -199,7 +197,7 @@ fn get_current_branch(repository: &Repository) -> Option<String> {
     let head = repository.head().ok()?;
     let shorthand = head.shorthand();
 
-    shorthand.map(|branch| branch.to_string())
+    shorthand.map(std::string::ToString::to_string)
 }
 
 #[cfg(test)]
diff --git a/src/init.rs b/src/init.rs
index 5db23c2b..396ba72d 100644
--- a/src/init.rs
+++ b/src/init.rs
@@ -54,8 +54,7 @@ pub fn init(shell_name: &str) {
         }
     };
 
-    if setup_script.is_some() {
-        let script = setup_script.unwrap();
+    if let Some(script) = setup_script {
         print!("{}", script);
     }
 }
diff --git a/src/module.rs b/src/module.rs
index 526345b9..ade5d9e5 100644
--- a/src/module.rs
+++ b/src/module.rs
@@ -17,13 +17,13 @@ pub struct Module<'a> {
     style: Style,
 
     /// The prefix used to separate the current module from the previous one.
-    prefix: ModuleAffix,
+    prefix: Affix,
 
     /// The collection of segments that compose this module.
     segments: Vec<Segment>,
 
     /// The suffix used to separate the current module from the next one.
-    suffix: ModuleAffix,
+    suffix: Affix,
 }
 
 impl<'a> Module<'a> {
@@ -33,9 +33,9 @@ impl<'a> Module<'a> {
             config,
             name: name.to_string(),
             style: Style::default(),
-            prefix: ModuleAffix::default_prefix(name.to_string()),
+            prefix: Affix::default_prefix(name),
             segments: Vec::new(),
-            suffix: ModuleAffix::default_suffix(name.to_string()),
+            suffix: Affix::default_suffix(name),
         }
     }
 
@@ -56,12 +56,12 @@ impl<'a> Module<'a> {
     }
 
     /// Get the module's prefix
-    pub fn get_prefix(&mut self) -> &mut ModuleAffix {
+    pub fn get_prefix(&mut self) -> &mut Affix {
         &mut self.prefix
     }
 
     /// Get the module's suffix
-    pub fn get_suffix(&mut self) -> &mut ModuleAffix {
+    pub fn get_suffix(&mut self) -> &mut Affix {
         &mut self.suffix
     }
 
@@ -82,7 +82,7 @@ impl<'a> Module<'a> {
         let mut ansi_strings = self
             .segments
             .iter()
-            .map(|s| s.ansi_string())
+            .map(Segment::ansi_string)
             .collect::<Vec<ANSIString>>();
 
         ansi_strings.insert(0, self.prefix.ansi_string());
@@ -119,7 +119,7 @@ impl<'a> fmt::Display for Module<'a> {
 }
 
 /// Module affixes are to be used for the prefix or suffix of a module.
-pub struct ModuleAffix {
+pub struct Affix {
     /// The affix's name, to be used in configuration and logging.
     name: String,
 
@@ -130,17 +130,17 @@ pub struct ModuleAffix {
     value: String,
 }
 
-impl ModuleAffix {
-    pub fn default_prefix(name: String) -> ModuleAffix {
-        ModuleAffix {
+impl Affix {
+    pub fn default_prefix(name: &str) -> Self {
+        Self {
             name: format!("{}_prefix", name),
             style: Style::default(),
             value: "via ".to_string(),
         }
     }
 
-    pub fn default_suffix(name: String) -> ModuleAffix {
-        ModuleAffix {
+    pub fn default_suffix(name: &str) -> Self {
+        Self {
             name: format!("{}_suffix", name),
             style: Style::default(),
             value: " ".to_string(),
@@ -150,7 +150,7 @@ impl ModuleAffix {
     /// Sets the style of the module.
     ///
     /// Accepts either `Color` or `Style`.
-    pub fn set_style<T>(&mut self, style: T) -> &mut ModuleAffix
+    pub fn set_style<T>(&mut self, style: T) -> &mut Self
     where
         T: Into<Style>,
     {
@@ -159,7 +159,7 @@ impl ModuleAffix {
     }
 
     /// Sets the value of the module.
-    pub fn set_value<T>(&mut self, value: T) -> &mut ModuleAffix
+    pub fn set_value<T>(&mut self, value: T) -> &mut Self
     where
         T: Into<String>,
     {
@@ -173,7 +173,7 @@ impl ModuleAffix {
     }
 }
 
-impl fmt::Display for ModuleAffix {
+impl fmt::Display for Affix {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(f, "{}", self.ansi_string())
     }
diff --git a/src/modules/directory.rs b/src/modules/directory.rs
index a14f1c20..0088b92c 100644
--- a/src/modules/directory.rs
+++ b/src/modules/directory.rs
@@ -33,12 +33,12 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
         // Contract the path to the git repo root
         let repo_folder_name = repo_root.file_name().unwrap().to_str().unwrap();
 
-        dir_string = contract_path(&current_dir, repo_root, repo_folder_name);
+        dir_string = contract_path(current_dir, repo_root, repo_folder_name);
     } else {
         // Contract the path to the home directory
         let home_dir = dirs::home_dir().unwrap();
 
-        dir_string = contract_path(&current_dir, &home_dir, HOME_SYMBOL);
+        dir_string = contract_path(current_dir, &home_dir, HOME_SYMBOL);
     }
 
     // Truncate the dir string to the maximum number of path components
@@ -89,8 +89,8 @@ fn replace_c_dir(path: String) -> String {
 ///
 /// On non-Windows OS, does nothing
 #[cfg(not(target_os = "windows"))]
-fn replace_c_dir(path: String) -> String {
-    return path;
+const fn replace_c_dir(path: String) -> String {
+    path
 }
 
 /// Truncate a path to only have a set number of path components
diff --git a/src/modules/git_branch.rs b/src/modules/git_branch.rs
index fc5601c5..e3a12e27 100644
--- a/src/modules/git_branch.rs
+++ b/src/modules/git_branch.rs
@@ -6,9 +6,9 @@ use super::{Context, Module};
 ///
 /// Will display the branch name if the current directory is a git repo
 pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
-    let branch_name = context.branch_name.as_ref()?;
-
     const GIT_BRANCH_CHAR: &str = " ";
+
+    let branch_name = context.branch_name.as_ref()?;
     let segment_color = Color::Purple.bold();
 
     let mut module = context.new_module("git_branch")?;
diff --git a/src/modules/git_status.rs b/src/modules/git_status.rs
index b56de363..8c675c33 100644
--- a/src/modules/git_status.rs
+++ b/src/modules/git_status.rs
@@ -40,11 +40,11 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
     module.get_suffix().set_value("] ").set_style(module_style);
     module.set_style(module_style);
 
-    let ahead_behind = get_ahead_behind(&repository, &branch_name);
-    if ahead_behind != Ok((0, 0)) {
-        log::debug!("Repo ahead/behind: {:?}", ahead_behind);
-    } else {
+    let ahead_behind = get_ahead_behind(&repository, branch_name);
+    if ahead_behind == Ok((0, 0)) {
         log::trace!("No ahead/behind found");
+    } else {
+        log::debug!("Repo ahead/behind: {:?}", ahead_behind);
     }
 
     let stash_object = repository.revparse_single("refs/stash");
diff --git a/src/modules/golang.rs b/src/modules/golang.rs
index 881c3015..17af7d5f 100644
--- a/src/modules/golang.rs
+++ b/src/modules/golang.rs
@@ -33,7 +33,7 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
             let mut module = context.new_module("go")?;
             module.set_style(module_color);
 
-            let formatted_version = format_go_version(go_version)?;
+            let formatted_version = format_go_version(&go_version)?;
             module.new_segment("symbol", GO_CHAR);
             module.new_segment("version", &formatted_version);
 
@@ -51,7 +51,7 @@ fn get_go_version() -> Option<String> {
         .and_then(|output| String::from_utf8(output.stdout).ok())
 }
 
-fn format_go_version(go_stdout: String) -> Option<String> {
+fn format_go_version(go_stdout: &str) -> Option<String> {
     let version = go_stdout
         // split into ["", "1.12.4 linux/amd64"]
         .splitn(2, "go version go")
@@ -74,7 +74,7 @@ mod tests {
 
     #[test]
     fn test_format_go_version() {
-        let input = String::from("go version go1.12 darwin/amd64");
+        let input = "go version go1.12 darwin/amd64";
         assert_eq!(format_go_version(input), Some("v1.12".to_string()));
     }
 }
diff --git a/src/modules/package.rs b/src/modules/package.rs
index 2912f2c0..044499f5 100644
--- a/src/modules/package.rs
+++ b/src/modules/package.rs
@@ -28,7 +28,7 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
 }
 
 fn extract_cargo_version(file_contents: &str) -> Option<String> {
-    let cargo_toml: toml::Value = toml::from_str(&file_contents).ok()?;
+    let cargo_toml: toml::Value = toml::from_str(file_contents).ok()?;
     let raw_version = cargo_toml.get("package")?.get("version")?.as_str()?;
 
     let formatted_version = format_version(raw_version);
@@ -36,7 +36,7 @@ fn extract_cargo_version(file_contents: &str) -> Option<String> {
 }
 
 fn extract_package_version(file_contents: &str) -> Option<String> {
-    let package_json: json::Value = json::from_str(&file_contents).ok()?;
+    let package_json: json::Value = json::from_str(file_contents).ok()?;
     let raw_version = package_json.get("version")?.as_str()?;
     if raw_version == "null" {
         return None;
diff --git a/src/modules/python.rs b/src/modules/python.rs
index 740928a4..fe643d4c 100644
--- a/src/modules/python.rs
+++ b/src/modules/python.rs
@@ -29,7 +29,7 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
             let mut module = context.new_module("python")?;
             module.set_style(module_color);
 
-            let formatted_version = format_python_version(python_version);
+            let formatted_version = format_python_version(&python_version);
             module.new_segment("symbol", PYTHON_CHAR);
             module.new_segment("version", &formatted_version);
 
@@ -57,7 +57,7 @@ fn get_python_version() -> Option<String> {
     }
 }
 
-fn format_python_version(python_stdout: String) -> String {
+fn format_python_version(python_stdout: &str) -> String {
     format!("v{}", python_stdout.trim_start_matches("Python ").trim())
 }
 
@@ -67,7 +67,7 @@ mod tests {
 
     #[test]
     fn test_format_python_version() {
-        let input = String::from("Python 3.7.2");
+        let input = "Python 3.7.2";
         assert_eq!(format_python_version(input), "v3.7.2");
     }
 }
diff --git a/src/modules/username.rs b/src/modules/username.rs
index 5b59a24b..59b5a3bd 100644
--- a/src/modules/username.rs
+++ b/src/modules/username.rs
@@ -7,9 +7,9 @@ use super::{Context, Module};
 /// Creates a module with the current user's username
 ///
 /// Will display the usename if any of the following criteria are met:
-///     - The current user isn't the same as the one that is logged in ($LOGNAME != $USER)
+///     - The current user isn't the same as the one that is logged in (`$LOGNAME` != `$USER`)
 ///     - The current user is root (UID = 0)
-///     - The user is currently connected as an SSH session ($SSH_CONNECTION)
+///     - The user is currently connected as an SSH session (`$SSH_CONNECTION`)
 pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
     let user = env::var("USER").ok();
     let logname = env::var("LOGNAME").ok();
diff --git a/src/segment.rs b/src/segment.rs
index 4e3a2c08..8419ec31 100644
--- a/src/segment.rs
+++ b/src/segment.rs
@@ -17,8 +17,8 @@ pub struct Segment {
 
 impl Segment {
     /// Creates a new segment with default fields.
-    pub fn new(name: &str) -> Segment {
-        Segment {
+    pub fn new(name: &str) -> Self {
+        Self {
             name: name.to_string(),
             style: None,
             value: "".to_string(),
@@ -28,7 +28,7 @@ impl Segment {
     /// Sets the style of the segment.
     ///
     /// Accepts either `Color` or `Style`.
-    pub fn set_style<T>(&mut self, style: T) -> &mut Segment
+    pub fn set_style<T>(&mut self, style: T) -> &mut Self
     where
         T: Into<Style>,
     {
@@ -37,7 +37,7 @@ impl Segment {
     }
 
     /// Sets the value of the segment.
-    pub fn set_value<T>(&mut self, value: T) -> &mut Segment
+    pub fn set_value<T>(&mut self, value: T) -> &mut Self
     where
         T: Into<String>,
     {