From aa894ace1fc4600dbc94bccb05532154ee2288d8 Mon Sep 17 00:00:00 2001
From: Geobert Quach <geobert@protonmail.com>
Date: Tue, 10 Sep 2019 16:17:11 +0100
Subject: [PATCH] refactor(args): Apply comments

---
 crates/ra_cli/src/help.rs   |  48 ++++---------
 crates/ra_cli/src/main.rs   | 140 ++++++++++++++++++------------------
 crates/ra_tools/src/help.rs |  27 +++----
 crates/ra_tools/src/main.rs |  64 ++++++++---------
 4 files changed, 117 insertions(+), 162 deletions(-)

diff --git a/crates/ra_cli/src/help.rs b/crates/ra_cli/src/help.rs
index bf31472acc0..5171578f001 100644
--- a/crates/ra_cli/src/help.rs
+++ b/crates/ra_cli/src/help.rs
@@ -1,6 +1,4 @@
-pub fn print_global_help() {
-    println!(
-        "ra-cli
+pub const GLOBAL_HELP: &str = "ra-cli
 
 USAGE:
     ra_cli <SUBCOMMAND>
@@ -13,13 +11,9 @@ SUBCOMMANDS:
     analysis-stats
     highlight
     parse
-    symbols"
-    )
-}
+    symbols";
 
-pub fn print_analysis_bench_help() {
-    println!(
-        "ra_cli-analysis-bench
+pub const ANALYSIS_BENCH_HELP: &str = "ra_cli-analysis-bench
 
 USAGE:
     ra_cli analysis-bench [FLAGS] [OPTIONS] [PATH]
@@ -33,13 +27,9 @@ OPTIONS:
     --highlight <PATH>               Hightlight this file
     
 ARGS:
-    <PATH>    Project to analyse"
-    )
-}
+    <PATH>    Project to analyse";
 
-pub fn print_analysis_stats_help() {
-    println!(
-        "ra-cli-analysis-stats
+pub const ANALYSIS_STATS_HELP: &str = "ra-cli-analysis-stats
 
 USAGE:
     ra_cli analysis-stats [FLAGS] [OPTIONS] [PATH]
@@ -53,44 +43,30 @@ OPTIONS:
     -o <ONLY>
     
 ARGS:
-    <PATH>"
-    )
-}
+    <PATH>";
 
-pub fn print_highlight_help() {
-    println!(
-        "ra-cli-highlight
+pub const HIGHLIGHT_HELP: &str = "ra-cli-highlight
     
 USAGE:
     ra_cli highlight [FLAGS]
     
 FLAGS:
     -h, --help       Prints help information
-    -r, --rainbow"
-    )
-}
+    -r, --rainbow";
 
-pub fn print_symbols_help() {
-    println!(
-        "ra-cli-symbols
+pub const SYMBOLS_HELP: &str = "ra-cli-symbols
     
 USAGE:
     ra_cli highlight [FLAGS]
     
 FLAGS:
-    -h, --help    Prints help inforamtion"
-    )
-}
+    -h, --help    Prints help inforamtion";
 
-pub fn print_parse_help() {
-    println!(
-        "ra-cli-parse
+pub const PARSE_HELP: &str = "ra-cli-parse
     
 USAGE:
     ra_cli parse [FLAGS]
     
 FLAGS:
     -h, --help       Prints help inforamtion
-        --no-dump"
-    )
-}
+        --no-dump";
diff --git a/crates/ra_cli/src/main.rs b/crates/ra_cli/src/main.rs
index a9a9fbddd0b..e6334cf569b 100644
--- a/crates/ra_cli/src/main.rs
+++ b/crates/ra_cli/src/main.rs
@@ -15,105 +15,101 @@ type Result<T> = std::result::Result<T, Box<dyn Error + Send + Sync>>;
 fn main() -> Result<()> {
     Logger::with_env().start()?;
 
-    let subcommand = std::env::args_os().nth(1);
-    if subcommand.is_none() {
-        help::print_global_help();
-        return Ok(());
-    }
-    let subcommand = subcommand.unwrap();
+    let subcommand = match std::env::args_os().nth(1) {
+        None => {
+            eprintln!("{}", help::GLOBAL_HELP);
+            return Ok(());
+        }
+        Some(s) => s,
+    };
     let mut matches = Arguments::from_vec(std::env::args_os().skip(2).collect());
 
     match &*subcommand.to_string_lossy() {
         "parse" => {
             if matches.contains(["-h", "--help"]) {
-                help::print_parse_help();
+                eprintln!("{}", help::PARSE_HELP);
                 return Ok(());
-            } else {
-                let no_dump = matches.contains("--no-dump");
-                matches.finish().or_else(handle_extra_flags)?;
-
-                let _p = profile("parsing");
-                let file = file()?;
-                if !no_dump {
-                    println!("{:#?}", file.syntax());
-                }
-                std::mem::forget(file);
             }
+            let no_dump = matches.contains("--no-dump");
+            matches.finish().or_else(handle_extra_flags)?;
+
+            let _p = profile("parsing");
+            let file = file()?;
+            if !no_dump {
+                println!("{:#?}", file.syntax());
+            }
+            std::mem::forget(file);
         }
         "symbols" => {
             if matches.contains(["-h", "--help"]) {
-                help::print_symbols_help();
+                eprintln!("{}", help::SYMBOLS_HELP);
                 return Ok(());
-            } else {
-                matches.finish().or_else(handle_extra_flags)?;
-                let file = file()?;
-                for s in file_structure(&file) {
-                    println!("{:?}", s);
-                }
+            }
+            matches.finish().or_else(handle_extra_flags)?;
+            let file = file()?;
+            for s in file_structure(&file) {
+                println!("{:?}", s);
             }
         }
         "highlight" => {
             if matches.contains(["-h", "--help"]) {
-                help::print_highlight_help();
+                eprintln!("{}", help::HIGHLIGHT_HELP);
                 return Ok(());
-            } else {
-                let rainbow_opt = matches.contains(["-r", "--rainbow"]);
-                matches.finish().or_else(handle_extra_flags)?;
-                let (analysis, file_id) = Analysis::from_single_file(read_stdin()?);
-                let html = analysis.highlight_as_html(file_id, rainbow_opt).unwrap();
-                println!("{}", html);
             }
+            let rainbow_opt = matches.contains(["-r", "--rainbow"]);
+            matches.finish().or_else(handle_extra_flags)?;
+            let (analysis, file_id) = Analysis::from_single_file(read_stdin()?);
+            let html = analysis.highlight_as_html(file_id, rainbow_opt).unwrap();
+            println!("{}", html);
         }
         "analysis-stats" => {
             if matches.contains(["-h", "--help"]) {
-                help::print_analysis_stats_help();
+                eprintln!("{}", help::ANALYSIS_STATS_HELP);
                 return Ok(());
-            } else {
-                let verbose = matches.contains(["-v", "--verbose"]);
-                let memory_usage = matches.contains("--memory-usage");
-                let path = matches.value_from_str("--path")?.unwrap_or("".to_string());
-                let only = matches.value_from_str(["-o", "--only"])?.map(|v: String| v.to_owned());
-                matches.finish().or_else(handle_extra_flags)?;
-                analysis_stats::run(
-                    verbose,
-                    memory_usage,
-                    path.as_ref(),
-                    only.as_ref().map(String::as_ref),
-                )?;
             }
+            let verbose = matches.contains(["-v", "--verbose"]);
+            let memory_usage = matches.contains("--memory-usage");
+            let path: String = matches.value_from_str("--path")?.unwrap_or_default();
+            let only = matches.value_from_str(["-o", "--only"])?.map(|v: String| v.to_owned());
+            matches.finish().or_else(handle_extra_flags)?;
+            analysis_stats::run(
+                verbose,
+                memory_usage,
+                path.as_ref(),
+                only.as_ref().map(String::as_ref),
+            )?;
         }
         "analysis-bench" => {
             if matches.contains(["-h", "--help"]) {
-                help::print_analysis_bench_help();
+                eprintln!("{}", help::ANALYSIS_BENCH_HELP);
                 return Ok(());
-            } else {
-                let verbose = matches.contains(["-v", "--verbose"]);
-                let path = matches.value_from_str("--path")?.unwrap_or("".to_string());
-                let highlight_path = matches.value_from_str("--highlight")?;
-                let complete_path = matches.value_from_str("--complete")?;
-                if highlight_path.is_some() && complete_path.is_some() {
-                    panic!("either --highlight or --complete must be set, not both")
-                }
-                let op = if let Some(path) = highlight_path {
-                    let path: String = path;
-                    analysis_bench::Op::Highlight { path: path.into() }
-                } else if let Some(path_line_col) = complete_path {
-                    let path_line_col: String = path_line_col;
-                    let (path_line, column) = rsplit_at_char(path_line_col.as_str(), ':')?;
-                    let (path, line) = rsplit_at_char(path_line, ':')?;
-                    analysis_bench::Op::Complete {
-                        path: path.into(),
-                        line: line.parse()?,
-                        column: column.parse()?,
-                    }
-                } else {
-                    panic!("either --highlight or --complete must be set")
-                };
-                matches.finish().or_else(handle_extra_flags)?;
-                analysis_bench::run(verbose, path.as_ref(), op)?;
             }
+            let verbose = matches.contains(["-v", "--verbose"]);
+            let path: String = matches.value_from_str("--path")?.unwrap_or_default();
+            let highlight_path = matches.value_from_str("--highlight")?;
+            let complete_path = matches.value_from_str("--complete")?;
+            if highlight_path.is_some() && complete_path.is_some() {
+                panic!("either --highlight or --complete must be set, not both")
+            }
+            let op = if let Some(path) = highlight_path {
+                let path: String = path;
+                analysis_bench::Op::Highlight { path: path.into() }
+            } else if let Some(path_line_col) = complete_path {
+                let path_line_col: String = path_line_col;
+                let (path_line, column) = rsplit_at_char(path_line_col.as_str(), ':')?;
+                let (path, line) = rsplit_at_char(path_line, ':')?;
+                analysis_bench::Op::Complete {
+                    path: path.into(),
+                    line: line.parse()?,
+                    column: column.parse()?,
+                }
+            } else {
+                panic!("either --highlight or --complete must be set")
+            };
+            matches.finish().or_else(handle_extra_flags)?;
+            analysis_bench::run(verbose, path.as_ref(), op)?;
         }
-        _ => help::print_global_help(),
+        _ => eprintln!("{}", help::GLOBAL_HELP),
     }
     Ok(())
 }
@@ -122,7 +118,7 @@ fn handle_extra_flags(e: pico_args::Error) -> Result<()> {
     if let pico_args::Error::UnusedArgsLeft(flags) = e {
         let mut invalid_flags = String::new();
         for flag in flags {
-            write!(&mut invalid_flags, "{}, ", flag).expect("Error on write");
+            write!(&mut invalid_flags, "{}, ", flag)?;
         }
         let (invalid_flags, _) = invalid_flags.split_at(invalid_flags.len() - 2);
         Err(format!("Invalid flags: {}", invalid_flags).into())
diff --git a/crates/ra_tools/src/help.rs b/crates/ra_tools/src/help.rs
index 5bfe657343a..6dde6c2d22f 100644
--- a/crates/ra_tools/src/help.rs
+++ b/crates/ra_tools/src/help.rs
@@ -1,6 +1,4 @@
-pub fn print_global_help() {
-    println!(
-        "tasks
+pub const GLOBAL_HELP: &str = "tasks
 
 USAGE:
     ra_tools <SUBCOMMAND>
@@ -15,13 +13,9 @@ SUBCOMMANDS:
     gen-syntax
     gen-tests
     install-ra
-    lint"
-    )
-}
+    lint";
 
-pub fn print_install_ra_help() {
-    println!(
-        "ra_tools-install-ra
+pub const INSTALL_RA_HELP: &str = "ra_tools-install-ra
 
 USAGE:
     ra_tools.exe install-ra [FLAGS]
@@ -30,12 +24,10 @@ FLAGS:
         --client-code
     -h, --help           Prints help information
         --jemalloc
-        --server"
-    )
-}
+        --server";
 
 pub fn print_no_param_subcommand_help(subcommand: &str) {
-    println!(
+    eprintln!(
         "ra_tools-{}
 
 USAGE:
@@ -47,10 +39,7 @@ FLAGS:
     );
 }
 
-pub fn print_install_ra_conflict() {
-    println!(
-        "error: The argument `--server` cannot be used with `--client-code`
+pub const INSTALL_RA_CONFLICT: &str =
+    "error: The argument `--server` cannot be used with `--client-code`
                     
-For more information try --help"
-    )
-}
+For more information try --help";
diff --git a/crates/ra_tools/src/main.rs b/crates/ra_tools/src/main.rs
index 5410edea9ab..f96f1875fa7 100644
--- a/crates/ra_tools/src/main.rs
+++ b/crates/ra_tools/src/main.rs
@@ -23,84 +23,78 @@ struct ServerOpt {
 }
 
 fn main() -> Result<()> {
-    let subcommand = std::env::args_os().nth(1);
-    if subcommand.is_none() {
-        help::print_global_help();
-        return Ok(());
-    }
-    let subcommand = subcommand.unwrap();
+    let subcommand = match std::env::args_os().nth(1) {
+        None => {
+            eprintln!("{}", help::GLOBAL_HELP);
+            return Ok(());
+        }
+        Some(s) => s,
+    };
     let mut matches = Arguments::from_vec(std::env::args_os().skip(2).collect());
     let subcommand = &*subcommand.to_string_lossy();
     match subcommand {
         "install-ra" | "install-code" => {
             if matches.contains(["-h", "--help"]) {
-                help::print_install_ra_help();
+                eprintln!("{}", help::INSTALL_RA_HELP);
                 return Ok(());
-            } else {
-                let server = matches.contains("--server");
-                let client_code = matches.contains("--client-code");
-                if server && client_code {
-                    help::print_install_ra_conflict();
-                    return Ok(());
-                }
-                let jemalloc = matches.contains("--jemalloc");
-                matches.finish().or_else(handle_extra_flags)?;
-                let opts = InstallOpt {
-                    client: if server { None } else { Some(ClientOpt::VsCode) },
-                    server: if client_code { None } else { Some(ServerOpt { jemalloc: jemalloc }) },
-                };
-                install(opts)?
             }
+            let server = matches.contains("--server");
+            let client_code = matches.contains("--client-code");
+            if server && client_code {
+                eprintln!("{}", help::INSTALL_RA_CONFLICT);
+                return Ok(());
+            }
+            let jemalloc = matches.contains("--jemalloc");
+            matches.finish().or_else(handle_extra_flags)?;
+            let opts = InstallOpt {
+                client: if server { None } else { Some(ClientOpt::VsCode) },
+                server: if client_code { None } else { Some(ServerOpt { jemalloc: jemalloc }) },
+            };
+            install(opts)?
         }
         "gen-tests" => {
             if matches.contains(["-h", "--help"]) {
                 help::print_no_param_subcommand_help(&subcommand);
                 return Ok(());
-            } else {
-                gen_tests(Overwrite)?
             }
+            gen_tests(Overwrite)?
         }
         "gen-syntax" => {
             if matches.contains(["-h", "--help"]) {
                 help::print_no_param_subcommand_help(&subcommand);
                 return Ok(());
-            } else {
-                generate_boilerplate(Overwrite)?
             }
+            generate_boilerplate(Overwrite)?
         }
         "format" => {
             if matches.contains(["-h", "--help"]) {
                 help::print_no_param_subcommand_help(&subcommand);
                 return Ok(());
-            } else {
-                run_rustfmt(Overwrite)?
             }
+            run_rustfmt(Overwrite)?
         }
         "format-hook" => {
             if matches.contains(["-h", "--help"]) {
                 help::print_no_param_subcommand_help(&subcommand);
                 return Ok(());
-            } else {
-                install_format_hook()?
             }
+            install_format_hook()?
         }
         "lint" => {
             if matches.contains(["-h", "--help"]) {
                 help::print_no_param_subcommand_help(&subcommand);
                 return Ok(());
-            } else {
-                run_clippy()?
             }
+            run_clippy()?
         }
         "fuzz-tests" => {
             if matches.contains(["-h", "--help"]) {
                 help::print_no_param_subcommand_help(&subcommand);
                 return Ok(());
-            } else {
-                run_fuzzer()?
             }
+            run_fuzzer()?
         }
-        _ => help::print_global_help(),
+        _ => eprintln!("{}", help::GLOBAL_HELP),
     }
     Ok(())
 }
@@ -109,7 +103,7 @@ fn handle_extra_flags(e: pico_args::Error) -> Result<()> {
     if let pico_args::Error::UnusedArgsLeft(flags) = e {
         let mut invalid_flags = String::new();
         for flag in flags {
-            write!(&mut invalid_flags, "{}, ", flag).expect("Error on write");
+            write!(&mut invalid_flags, "{}, ", flag)?;
         }
         let (invalid_flags, _) = invalid_flags.split_at(invalid_flags.len() - 2);
         Err(format!("Invalid flags: {}", invalid_flags).into())