Repository: dotenv-linter/dotenv-linter (rust)

Issue:
Proposal: Using subcommands intead of flags
### Problems
1. We now have a few flags, which are essentially subcommands: `--fix` & `--show-checks`;
2. Some flags don't work with other. Example: `$ dotenv-linter --no-backup`;
3. Adding new commands like `compare` confuses the code even more.

### Solution
**Breaking changes**: Using subcommands intead of flags.

Checking `.env` files will by default have no subcommand:

```bash
$ dotenv-linter
```

Fixing `.env` files will have the `fix` subcommand:

```bash
$ dotenv-linter fix
```

Showing checks will have the `checks` subcommand:

```bash
$ dotenv-linter checks
```

### Additional changes
Move to the [clap](https://github.com/clap-rs/clap) `v3.0.0-beta.2` - there are a lot of new changes like:
* Generate a CLI simply by defining a struct;
* Auto-generated completion scripts (using `clap_generate`);
* Derive macros;
* Support for building CLIs from YAML;
* Automatic Version from `Cargo.toml`.

What do you think?

Pull request:
Remade flags to subcommands
Close #333 

What was done:
- [x] Remade the `--fix/-f` flag to the `fix` subcommand;
- [x] Remade the `--show-checks` flag to the `list` subcommand;
- [x] Fixed a bug with output for multiple files in quiet mode (and wrote a test for that case);
- [x] Added colored help output.

<!--
Thank you for your pull request. Please provide a description above and review
the requirements below.

Bug fixes and new features should include tests.
-->

<!-- _Please make sure to review and check all of these items:_ -->

#### ✔ Checklist:
<!-- Remove items that do not apply. For completed items, change [ ] to [x]. -->

- [x] This PR has been added to [CHANGELOG.md](https://github.com/dotenv-linter/dotenv-linter/blob/master/CHANGELOG.md) (at the top of the list);
- [x] Tests for the changes have been added (for bug fixes / features);
- [ ] Docs have been added / updated (for bug fixes / features).

<!-- _NOTE: these things are not required to open a PR and can be done afterwards / while the PR is open._ -->


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1e65225..2c4f144 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@ ### 🚀 Added
 - Add export prefix support [#340](https://github.com/dotenv-linter/dotenv-linter/pull/340)([@skonik](https://github.com/skonik))
 
 ### 🔧 Changed
+- Remade flags to subcommands [#342](https://github.com/dotenv-linter/dotenv-linter/pull/342) ([@mgrachev](https://github.com/mgrachev))
 - Changed behavior of QuoteCharacterChecker for multiline values support [#341](https://github.com/dotenv-linter/dotenv-linter/pull/341) ([@artem-russkikh](http://github.com/artem-russkikh))
 - Make an output on-the-fly [#336](https://github.com/dotenv-linter/dotenv-linter/pull/336) ([@DDtKey](https://github.com/DDtKey))
 
diff --git a/Cargo.lock b/Cargo.lock
index 6cb3892..2de9c35 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -46,9 +46,9 @@ checksum = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822"
 
 [[package]]
 name = "clap"
-version = "2.33.0"
+version = "2.33.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5067f5bb2d80ef5d68b4c87db81601f0b75bca627bc2ef76b141d7b846a3c6d9"
+checksum = "37e58ac78573c40708d45522f0d80fa2f01cc4f9b4e2bf749807255454312002"
 dependencies = [
  "ansi_term",
  "atty",
diff --git a/Cargo.toml b/Cargo.toml
index 3787995..91c3860 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -11,7 +11,7 @@ keywords = ["environment", "env", "dotenv", "linter", "lint"]
 categories = ["command-line-utilities"]
 
 [dependencies]
-clap = "2.33.0"
+clap = "2.33.3"
 colored = "2.0.0"
 
 [target.'cfg(windows)'.dependencies]
diff --git a/src/common/output/check.rs b/src/common/output/check.rs
index 797df24..94328af 100644
--- a/src/common/output/check.rs
+++ b/src/common/output/check.rs
@@ -18,14 +18,19 @@ pub fn new(is_quiet_mode: bool, files_count: usize) -> Self {
 
     /// Prints information about a file in process
     pub fn print_processing_info(&self, file: &FileEntry) {
-        if !&self.is_quiet_mode {
+        if !self.is_quiet_mode {
             println!("Checking {}", file);
         }
     }
 
-    /// Prints warnings without any additional information.
+    /// Prints warnings without any additional information
     pub fn print_warnings(&self, warnings: &[Warning], file_index: usize) {
         warnings.iter().for_each(|w| println!("{}", w));
+
+        if self.is_quiet_mode {
+            return;
+        }
+
         let is_last_file = file_index == self.files_count - 1;
         if !warnings.is_empty() && !is_last_file {
             println!();
diff --git a/src/common/output/fix.rs b/src/common/output/fix.rs
index 4e84ff7..10b6646 100644
--- a/src/common/output/fix.rs
+++ b/src/common/output/fix.rs
@@ -1,22 +1,27 @@
-use crate::common::FileEntry;
+use crate::common::{FileEntry, Warning};
 use std::ffi::OsString;
 
-/// Prefix for the backup output.
+/// Prefix for the backup output
 const BACKUP_PREFIX: &str = "Original file was backed up to: ";
 
 pub struct FixOutput {
     // Quiet program output mode
     is_quiet_mode: bool,
+    // Total number of files to check
+    files_count: usize,
 }
 
 impl FixOutput {
-    pub fn new(is_quiet_mode: bool) -> Self {
-        FixOutput { is_quiet_mode }
+    pub fn new(is_quiet_mode: bool, files_count: usize) -> Self {
+        FixOutput {
+            is_quiet_mode,
+            files_count,
+        }
     }
 
     /// Prints information about a file in process
     pub fn print_processing_info(&self, file: &FileEntry) {
-        if !&self.is_quiet_mode {
+        if !self.is_quiet_mode {
             println!("Fixing {}", file);
         }
     }
@@ -29,11 +34,24 @@ pub fn print_total(&self, total: usize) {
         }
     }
 
-    /// Prints the backup file's path.
+    /// Prints the backup file's path
     pub fn print_backup(&self, backup_path: &OsString) {
         println!("{}{:?}", BACKUP_PREFIX, backup_path);
         if !self.is_quiet_mode {
             println!();
         }
     }
+
+    /// Prints warnings without any additional information
+    pub fn print_warnings(&self, warnings: &[Warning], file_index: usize) {
+        if self.is_quiet_mode {
+            return;
+        }
+
+        warnings.iter().for_each(|w| println!("{}", w));
+        let is_last_file = file_index == self.files_count - 1;
+        if !warnings.is_empty() && !is_last_file {
+            println!();
+        }
+    }
 }
diff --git a/src/lib.rs b/src/lib.rs
index 5ddf16e..16d79d3 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1,5 +1,5 @@
 use crate::common::*;
-
+use std::collections::BTreeMap;
 use std::error::Error;
 use std::path::PathBuf;
 
@@ -10,13 +10,12 @@
 
 pub use checks::available_check_names;
 
-#[allow(clippy::redundant_closure)]
-pub fn run(args: &clap::ArgMatches, current_dir: &PathBuf) -> Result<usize, Box<dyn Error>> {
+pub fn check(args: &clap::ArgMatches, current_dir: &PathBuf) -> Result<usize, Box<dyn Error>> {
     let mut warnings_count = 0;
-    let file_paths: Vec<PathBuf> = get_needed_file_paths(args);
+    let lines_map = get_lines(args, current_dir)?;
 
-    // Nothing to check/fix
-    if file_paths.is_empty() {
+    // Nothing to check
+    if lines_map.is_empty() {
         return Ok(warnings_count);
     }
 
@@ -25,60 +24,91 @@ pub fn run(args: &clap::ArgMatches, current_dir: &PathBuf) -> Result<usize, Box<
         skip_checks = skip.collect();
     }
 
-    let is_fix = args.is_present("fix");
-    let is_quiet_mode = args.is_present("quiet");
+    let output = CheckOutput::new(args.is_present("quiet"), lines_map.len());
+    for (index, (fe, strings)) in lines_map.into_iter().enumerate() {
+        output.print_processing_info(&fe);
 
-    let fix_output = FixOutput::new(is_quiet_mode);
-    let check_output = CheckOutput::new(is_quiet_mode, file_paths.len());
+        let lines = get_line_entries(&fe, strings);
+        let result = checks::run(&lines, &skip_checks);
 
-    for (i, path) in file_paths.iter().enumerate() {
-        let relative_path = match fs_utils::get_relative_path(&path, &current_dir) {
-            Some(p) => p,
-            None => continue,
-        };
+        output.print_warnings(&result, index);
+        warnings_count += result.len();
+    }
 
-        let (fe, strs) = match FileEntry::from(relative_path) {
-            Some(f) => f,
-            None => continue,
-        };
+    output.print_total(warnings_count);
 
-        if is_fix {
-            fix_output.print_processing_info(&fe);
-        } else {
-            check_output.print_processing_info(&fe);
-        }
+    Ok(warnings_count)
+}
 
-        let mut lines = get_line_entries(&fe, strs);
+pub fn fix(args: &clap::ArgMatches, current_dir: &PathBuf) -> Result<(), Box<dyn Error>> {
+    let mut warnings_count = 0;
+    let lines_map = get_lines(args, current_dir)?;
+
+    // Nothing to fix
+    if lines_map.is_empty() {
+        return Ok(());
+    }
+
+    let mut skip_checks: Vec<&str> = Vec::new();
+    if let Some(skip) = args.values_of("skip") {
+        skip_checks = skip.collect();
+    }
 
+    let output = FixOutput::new(args.is_present("quiet"), lines_map.len());
+    for (index, (fe, strings)) in lines_map.into_iter().enumerate() {
+        output.print_processing_info(&fe);
+
+        let mut lines = get_line_entries(&fe, strings);
         let mut result = checks::run(&lines, &skip_checks);
+
         // run fixers & write results to file
-        if is_fix && !result.is_empty() && fixes::run(&mut result, &mut lines, &skip_checks) > 0 {
+        if !result.is_empty() && fixes::run(&mut result, &mut lines, &skip_checks) > 0 {
             let should_backup = !args.is_present("no-backup");
             // create backup copy unless user specifies not to
             if should_backup {
                 let backup_file = fs_utils::backup_file(&fe)?.into_os_string();
-                fix_output.print_backup(&backup_file);
+                output.print_backup(&backup_file);
             }
 
             // write corrected file
             fs_utils::write_file(&fe.path, lines)?;
         }
 
-        // This shouldn't be printed to Fix when combined with quiet mode
-        if !(is_fix && is_quiet_mode) {
-            check_output.print_warnings(&result, i);
-        }
-
+        output.print_warnings(&result, index);
         warnings_count += result.len();
     }
 
-    if is_fix {
-        fix_output.print_total(warnings_count);
-    } else {
-        check_output.print_total(warnings_count);
+    output.print_total(warnings_count);
+
+    Ok(())
+}
+
+fn get_lines(
+    args: &clap::ArgMatches,
+    current_dir: &PathBuf,
+) -> Result<BTreeMap<FileEntry, Vec<String>>, Box<dyn Error>> {
+    let mut lines: BTreeMap<FileEntry, Vec<String>> = BTreeMap::new();
+    let file_paths: Vec<PathBuf> = get_needed_file_paths(args);
+
+    if file_paths.is_empty() {
+        return Ok(lines);
     }
 
-    Ok(warnings_count)
+    for path in file_paths.iter() {
+        let relative_path = match fs_utils::get_relative_path(&path, &current_dir) {
+            Some(p) => p,
+            None => continue,
+        };
+
+        let (fe, strings) = match FileEntry::from(relative_path) {
+            Some(f) => f,
+            None => continue,
+        };
+
+        lines.insert(fe, strings);
+    }
+
+    Ok(lines)
 }
 
 /// getting a list of all files for checking/fixing without custom exclusion files
diff --git a/src/main.rs b/src/main.rs
index 7b9d661..7440f77 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,31 +1,41 @@
-use clap::Arg;
+use clap::{AppSettings, Arg, SubCommand};
 use std::error::Error;
 use std::ffi::OsStr;
 use std::{env, process};
 
 fn main() -> Result<(), Box<dyn Error>> {
+    #[cfg(windows)]
+    colored::control::set_virtual_terminal(true).ok();
+
     let current_dir = env::current_dir()?;
     let args = get_args(current_dir.as_os_str());
 
-    if args.is_present("show-checks") {
-        dotenv_linter::available_check_names()
-            .iter()
-            .for_each(|name| println!("{}", name));
-        process::exit(0);
-    }
-
-    let no_color = args.is_present("no-color");
-    if no_color {
+    if args.is_present("no-color") {
         colored::control::set_override(false);
     }
-    #[cfg(windows)]
-    set_windows_virtual_terminal();
 
-    let total_warnings = dotenv_linter::run(&args, &current_dir)?;
+    match args.subcommand() {
+        ("", None) => {
+            let total_warnings = dotenv_linter::check(&args, &current_dir)?;
 
-    // Ensure the exit code is 0 if there are no warnings or have been fixed.
-    if args.is_present("fix") || total_warnings == 0 {
-        process::exit(0);
+            if total_warnings == 0 {
+                process::exit(0);
+            }
+        }
+        ("fix", Some(fix_args)) => {
+            dotenv_linter::fix(&fix_args, &current_dir)?;
+            process::exit(0);
+        }
+        ("list", Some(_)) => {
+            dotenv_linter::available_check_names()
+                .iter()
+                .for_each(|name| println!("{}", name));
+
+            process::exit(0);
+        }
+        _ => {
+            eprintln!("unknown command");
+        }
     }
 
     process::exit(1);
@@ -33,73 +43,69 @@ fn main() -> Result<(), Box<dyn Error>> {
 
 fn get_args(current_dir: &OsStr) -> clap::ArgMatches {
     clap::App::new(env!("CARGO_PKG_NAME"))
+        .setting(AppSettings::ColoredHelp)
+        .setting(AppSettings::DisableHelpSubcommand)
+        .setting(AppSettings::VersionlessSubcommands)
         .about(env!("CARGO_PKG_DESCRIPTION"))
         .author(env!("CARGO_PKG_AUTHORS"))
         .version(env!("CARGO_PKG_VERSION"))
         .version_short("v")
-        .arg(
-            Arg::with_name("input")
-                .help("files or paths")
-                .index(1)
-                .default_value_os(current_dir)
-                .required(true)
-                .multiple(true),
-        )
-        .arg(
-            Arg::with_name("exclude")
-                .short("e")
-                .long("exclude")
-                .value_name("FILE_NAME")
-                .help("Excludes files from check")
-                .multiple(true)
-                .takes_value(true),
+        .args(common_args(current_dir).as_ref())
+        .subcommand(
+            SubCommand::with_name("list")
+                .setting(AppSettings::ColoredHelp)
+                .visible_alias("l")
+                .usage("dotenv-linter list")
+                .about("Shows list of available checks"),
         )
-        .arg(
-            Arg::with_name("skip")
-                .short("s")
-                .long("skip")
-                .value_name("CHECK_NAME")
-                .help("Skips checks")
-                .multiple(true)
-                .takes_value(true),
-        )
-        .arg(
-            Arg::with_name("show-checks")
-                .long("show-checks")
-                .help("Shows list of available checks"),
-        )
-        .arg(
-            Arg::with_name("recursive")
-                .short("r")
-                .long("recursive")
-                .help("Recursively search and check .env files"),
-        )
-        .arg(
-            Arg::with_name("no-color")
-                .long("no-color")
-                .help("Turns off the colored output"),
-        )
-        .arg(
-            Arg::with_name("fix")
-                .short("f")
-                .long("fix")
-                .help("Automatically fixes warnings"),
-        )
-        .arg(
-            Arg::with_name("no-backup")
-                .long("no-backup")
-                .help("Prevents .env files from being backed up when modified by -f/--fix"),
-        )
-        .arg(
-            Arg::with_name("quiet")
-                .short("q")
-                .long("quiet")
-                .help("Doesn't display additional information"),
+        .subcommand(
+            SubCommand::with_name("fix")
+                .setting(AppSettings::ColoredHelp)
+                .visible_alias("f")
+                .args(common_args(current_dir).as_ref())
+                .arg(
+                    Arg::with_name("no-backup")
+                        .long("no-backup")
+                        .help("Prevents backing up .env files"),
+                )
+                .usage("dotenv-linter fix [FLAGS] [OPTIONS] <input>...")
+                .about("Automatically fixes warnings"),
         )
         .get_matches()
 }
 
-#[cfg(windows)]
-pub fn set_windows_virtual_terminal() {
-    colored::control::set_virtual_terminal(true).ok();
+fn common_args(current_dir: &OsStr) -> Vec<Arg> {
+    vec![
+        Arg::with_name("input")
+            .help("files or paths")
+            .index(1)
+            .default_value_os(current_dir)
+            .required(true)
+            .multiple(true),
+        Arg::with_name("exclude")
+            .short("e")
+            .long("exclude")
+            .value_name("FILE_NAME")
+            .help("Excludes files from check")
+            .multiple(true)
+            .takes_value(true),
+        Arg::with_name("skip")
+            .short("s")
+            .long("skip")
+            .value_name("CHECK_NAME")
+            .help("Skips checks")
+            .multiple(true)
+            .takes_value(true),
+        Arg::with_name("recursive")
+            .short("r")
+            .long("recursive")
+            .help("Recursively searches and checks .env files"),
+        Arg::with_name("no-color")
+            .long("no-color")
+            .help("Turns off the colored output"),
+        Arg::with_name("quiet")
+            .short("q")
+            .long("quiet")
+            .help("Doesn't display additional information"),
+    ]
 }


Test patch:
diff --git a/tests/common/test_dir.rs b/tests/common/test_dir.rs
index 84acbd0..6e202e2 100644
--- a/tests/common/test_dir.rs
+++ b/tests/common/test_dir.rs
@@ -130,7 +130,7 @@ pub fn test_command_fix_success(&self, expected_output: String) {
         let mut cmd = Self::init_cmd();
         let canonical_current_dir = canonicalize(&self.current_dir).expect("canonical current dir");
         cmd.current_dir(&canonical_current_dir)
-            .args(&["-f", "--no-backup"])
+            .args(&["fix", "--no-backup"])
             .assert()
             .success()
             .stdout(expected_output);
@@ -143,7 +143,7 @@ pub fn test_command_fix_success_without_output(&self) {
         let mut cmd = Self::init_cmd();
         let canonical_current_dir = canonicalize(&self.current_dir).expect("canonical current dir");
         cmd.current_dir(&canonical_current_dir)
-            .args(&["-f", "--no-backup"])
+            .args(&["fix", "--no-backup"])
             .assert()
             .success();
     }
@@ -160,7 +160,7 @@ pub fn test_command_fix_success_with_args<I, S>(&self, expected_output: String,
         let mut cmd = Self::init_cmd();
         let canonical_current_dir = canonicalize(&self.current_dir).expect("canonical current dir");
         cmd.current_dir(&canonical_current_dir)
-            .args(&["-f", "--no-backup"])
+            .args(&["fix", "--no-backup"])
             .args(ext_args)
             .assert()
             .success()
diff --git a/tests/flags/backup.rs b/tests/flags/backup.rs
index 7ec8594..8af6ac2 100644
--- a/tests/flags/backup.rs
+++ b/tests/flags/backup.rs
@@ -7,7 +7,7 @@ fn output_backup_file() {
     let testdir = TestDir::new();
     let content = "foo=bar\n";
     let testfile = testdir.create_testfile(".env", content);
-    let args = &[testfile.as_str(), "-f"];
+    let args = &[testfile.as_str(), "fix"];
 
     testdir.test_command_success_with_args_without_closing(args);
 
diff --git a/tests/flags/quiet.rs b/tests/flags/quiet.rs
index c96afa3..bcbbbbf 100644
--- a/tests/flags/quiet.rs
+++ b/tests/flags/quiet.rs
@@ -14,6 +14,22 @@ fn check_output_in_quiet_mode() {
     test_dir.test_command_fail_with_args(args, expected_output);
 }
 
+#[test]
+fn check_output_for_multiple_files_in_quiet_mode() {
+    let test_dir = TestDir::new();
+    let testfile_1 = test_dir.create_testfile(".env", "BAR='Baz'\n");
+    let testfile_2 = test_dir.create_testfile(".env2", " BAR=\n");
+
+    let args = &["--quiet"];
+    let expected_output = format!(
+        "{a}:1 QuoteCharacter: The value has quote characters (\', \")\n{b}:1 LeadingCharacter: Invalid leading character detected\n",
+        a=testfile_1.shortname_as_str(),
+        b=testfile_2.shortname_as_str()
+    );
+
+    test_dir.test_command_fail_with_args(args, expected_output);
+}
+
 #[test]
 fn fix_output_in_quiet_mode() {
     let test_dir = TestDir::new();
diff --git a/tests/output/fix.rs b/tests/output/fix.rs
index 24b4f4e..9acd079 100644
--- a/tests/output/fix.rs
+++ b/tests/output/fix.rs
@@ -176,7 +176,7 @@ fn backup() {
     let test_dir = TestDir::new();
     let test_file = test_dir.create_testfile(".env", "abc=DEF\n\nF=BAR\nB=bbb\n");
 
-    let args = &["-f"];
+    let args = &["fix"];
     let output = test_dir.test_command_success_and_get_output(args);
 
     let backup_file = fs::read_dir(&test_dir.as_str())
@@ -210,7 +210,7 @@ fn quiet_backup() {
     let test_dir = TestDir::new();
     let test_file = test_dir.create_testfile(".env", "abc=DEF\n\nF=BAR\nB=bbb\n");
 
-    let args = &["-f", "-q"];
+    let args = &["fix", "-q"];
     let output = test_dir.test_command_success_and_get_output(args);
 
     let backup_file = fs::read_dir(&test_dir.as_str())