apply review comments

src/cobertura.rs

@@ -7,12 +7,11 @@ use std::path::Path;
 use std::time::{SystemTime, UNIX_EPOCH};
 use std::{
     io::{BufWriter, Cursor, Write},
-    path::PathBuf,
 };
 use symbolic_common::Name;
 use symbolic_demangle::{Demangle, DemangleOptions};
 
-use crate::{output::get_target_output_writable, CovResult};
+use crate::{output::get_target_output_writable, ResultTuple};
 
 macro_rules! demangle {
     ($name: expr, $demangle: expr, $options: expr) => {{
@@ -231,7 +230,7 @@ impl ToString for ConditionType {
 }
 
 fn get_coverage(
-    results: &[(PathBuf, PathBuf, CovResult)],
+    results: &[ResultTuple],
     sources: Vec<String>,
     demangle: bool,
     demangle_options: DemangleOptions,
@@ -329,8 +328,8 @@ fn get_coverage(
 
 pub fn output_cobertura(
     source_dir: Option<&Path>,
-    results: &[(PathBuf, PathBuf, CovResult)],
-    output_path: Option<&Path>,
+    results: &[ResultTuple],
+    output_file: Option<&Path>,
     demangle: bool,
 ) {
     let demangle_options = DemangleOptions::name_only();
@@ -491,14 +490,7 @@ pub fn output_cobertura(
         .unwrap();
 
     let result = writer.into_inner().into_inner();
-    let output_file = output_path.map(|path| {
-        if path.is_dir() {
-            path.join("cobertura.xml")
-        } else {
-            path.to_path_buf()
-        }
-    });
-    let mut file = BufWriter::new(get_target_output_writable(output_file.as_ref()));
+    let mut file = BufWriter::new(get_target_output_writable(output_file));
     file.write_all(&result).unwrap();
 }
 

src/lib.rs

@@ -44,12 +44,15 @@ pub use crate::file_filter::*;
 use log::{error, warn};
 use std::fs;
 use std::io::{BufReader, Cursor};
+use std::path::PathBuf;
 use std::{
     collections::{btree_map, hash_map},
     path::Path,
 };
 use walkdir::WalkDir;
 
+pub type ResultTuple = (PathBuf, PathBuf, CovResult);  
+
 // Merge results, without caring about duplicate lines (they will be removed at the end).
 pub fn merge_results(result: &mut CovResult, result2: CovResult) -> bool {
     let mut warn_overflow = false;

src/main.rs

@@ -50,6 +50,27 @@ impl FromStr for OutputType {
     }
 }
 
+impl OutputType {
+    fn to_file_name(&self, output_path: Option<&Path>) -> Option<PathBuf> {
+        output_path.map(|path| {
+            if path.is_dir() {
+                match self {
+                    OutputType::Ade => path.join("activedata"),
+                    OutputType::Lcov => path.join("lcov"),
+                    OutputType::Coveralls => path.join("coveralls"),
+                    OutputType::CoverallsPlus => path.join("coveralls+"),
+                    OutputType::Files => path.join("files"),
+                    OutputType::Covdir => path.join("covdir"),
+                    OutputType::Html => path.join("html"),
+                    OutputType::Cobertura => path.join("cobertura.xml"),
+                }
+            } else {
+                path.to_path_buf()
+            }
+        })
+    }
+}
+
 enum Filter {
     Covered,
     Uncovered,
@@ -108,12 +129,13 @@ struct Opt {
         use_delimiter = true
     )]
     output_types: Vec<OutputType>,
-    /// Specifies the output path.
-    #[structopt(short, long, value_name = "PATH", alias = "output-folder")]
+    /// Specifies the output path. This is a file for a single output type and must be a folder
+    /// for multiple output types.
+    #[structopt(short, long, value_name = "PATH", alias = "output-file")]
     output_path: Option<PathBuf>,
     /// Specifies the output config file.
-    #[structopt(long, value_name = "PATH", alias = "html-output-config-file")]
-    html_output_config_file: Option<PathBuf>,
+    #[structopt(long, value_name = "PATH", alias = "output-config-file")]
+    output_config_file: Option<PathBuf>,
     /// Specifies the root directory of the source files.
     #[structopt(short, long, value_name = "DIRECTORY", parse(from_os_str))]
     source_dir: Option<PathBuf>,
@@ -434,9 +456,11 @@ fn main() {
     };
 
     for output_type in &opt.output_types {
+        let output_path = output_type.to_file_name(output_path);
+
         match output_type {
-            OutputType::Ade => output_activedata_etl(&iterator, output_path, demangle),
-            OutputType::Lcov => output_lcov(&iterator, output_path, demangle),
+            OutputType::Ade => output_activedata_etl(&iterator, output_path.as_deref(), demangle),
+            OutputType::Lcov => output_lcov(&iterator, output_path.as_deref(), demangle),
             OutputType::Coveralls => output_coveralls(
                 &iterator,
                 opt.token.as_deref(),
@@ -446,7 +470,7 @@ fn main() {
                 &service_pull_request,
                 &commit_sha,
                 false,
-                output_path,
+                output_path.as_deref(),
                 &opt.vcs_branch,
                 opt.parallel,
                 demangle,
@@ -460,24 +484,27 @@ fn main() {
                 &service_pull_request,
                 &commit_sha,
                 true,
-                output_path,
+                output_path.as_deref(),
                 &opt.vcs_branch,
                 opt.parallel,
                 demangle,
             ),
-            OutputType::Files => output_files(&iterator, output_path),
-            OutputType::Covdir => output_covdir(&iterator, output_path),
+            OutputType::Files => output_files(&iterator, output_path.as_deref()),
+            OutputType::Covdir => output_covdir(&iterator, output_path.as_deref()),
             OutputType::Html => output_html(
                 &iterator,
-                output_path,
+                output_path.as_deref(),
                 num_threads,
                 opt.branch,
-                opt.html_output_config_file.as_deref(),
+                opt.output_config_file.as_deref(),
+            ),
+            OutputType::Cobertura => output_cobertura(
+                source_root.as_deref(),
+                &iterator,
+                output_path.as_deref(),
+                demangle,
             ),
-            OutputType::Cobertura => {
-                output_cobertura(source_root.as_deref(), &iterator, output_path, demangle)
-            },
-            OutputType::Markdown => output_markdown(&iterator, opt.output_path.as_deref()),
+            OutputType::Markdown => output_markdown(&iterator, output_path.as_deref()),
         };
     }
 }

src/output.rs

@@ -19,7 +19,7 @@ use symbolic_demangle::{Demangle, DemangleOptions};
 use tabled::{Style, Table, Tabled};
 use uuid::Uuid;
 
-use crate::defs::*;
+use crate::{defs::*, ResultTuple};
 use crate::html;
 
 macro_rules! demangle {
@@ -36,7 +36,7 @@ macro_rules! demangle {
     }};
 }
 
-pub fn get_target_output_writable(output_file: Option<&PathBuf>) -> Box<dyn Write> {
+pub fn get_target_output_writable(output_file: Option<&Path>) -> Box<dyn Write> {
     let write_target: Box<dyn Write> = match output_file {
         Some(output) => {
             if output.is_dir() {
@@ -71,19 +71,12 @@ pub fn get_target_output_writable(output_file: Option<&PathBuf>) -> Box<dyn Writ
 }
 
 pub fn output_activedata_etl(
-    results: &[(PathBuf, PathBuf, CovResult)],
-    output_path: Option<&Path>,
+    results: &[ResultTuple],
+    output_file: Option<&Path>,
     demangle: bool,
 ) {
     let demangle_options = DemangleOptions::name_only();
-    let output_file = output_path.map(|path| {
-        if path.is_dir() {
-            path.join("activedata")
-        } else {
-            path.to_path_buf()
-        }
-    });
-    let mut writer = BufWriter::new(get_target_output_writable(output_file.as_ref()));
+    let mut writer = BufWriter::new(get_target_output_writable(output_file));
 
     for (_, rel_path, result) in results {
         let covered: Vec<u32> = result
@@ -191,15 +184,8 @@ pub fn output_activedata_etl(
     }
 }
 
-pub fn output_covdir(results: &[(PathBuf, PathBuf, CovResult)], output_path: Option<&Path>) {
-    let output_file = output_path.map(|path| {
-        if path.is_dir() {
-            path.join("covdir")
-        } else {
-            path.to_path_buf()
-        }
-    });
-    let mut writer = BufWriter::new(get_target_output_writable(output_file.as_ref()));
+pub fn output_covdir(results: &[ResultTuple], output_file: Option<&Path>) {
+    let mut writer = BufWriter::new(get_target_output_writable(output_file));
     let mut relative: FxHashMap<PathBuf, Rc<RefCell<CDDirStats>>> = FxHashMap::default();
     let global = Rc::new(RefCell::new(CDDirStats::new("".to_string())));
     relative.insert(PathBuf::from(""), global.clone());
@@ -255,19 +241,12 @@ pub fn output_covdir(results: &[(PathBuf, PathBuf, CovResult)], output_path: Opt
 }
 
 pub fn output_lcov(
-    results: &[(PathBuf, PathBuf, CovResult)],
-    output_path: Option<&Path>,
+    results: &[ResultTuple],
+    output_file: Option<&Path>,
     demangle: bool,
 ) {
     let demangle_options = DemangleOptions::name_only();
-    let output_file = output_path.map(|path| {
-        if path.is_dir() {
-            path.join("lcov")
-        } else {
-            path.to_path_buf()
-        }
-    });
-    let mut writer = BufWriter::new(get_target_output_writable(output_file.as_ref()));
+    let mut writer = BufWriter::new(get_target_output_writable(output_file));
     writer.write_all(b"TN:\n").unwrap();
 
     for (_, rel_path, result) in results {
@@ -442,15 +421,15 @@ fn get_coveralls_git_info(commit_sha: &str, vcs_branch: &str) -> Value {
 }
 
 pub fn output_coveralls(
-    results: &[(PathBuf, PathBuf, CovResult)],
+    results: &[ResultTuple],
     repo_token: Option<&str>,
     service_name: Option<&str>,
     service_number: &str,
     service_job_id: Option<&str>,
     service_pull_request: &str,
     commit_sha: &str,
     with_function_info: bool,
-    output_path: Option<&Path>,
+    output_file: Option<&Path>,
     vcs_branch: &str,
     parallel: bool,
     demangle: bool,
@@ -530,36 +509,19 @@ pub fn output_coveralls(
         obj.insert("service_job_id".to_string(), json!(service_job_id));
     }
 
-    let output_file = output_path.map(|path| {
-        if path.is_dir() {
-            path.join(format!(
-                "coveralls{}",
-                if with_function_info { "+" } else { "" }
-            ))
-        } else {
-            path.to_path_buf()
-        }
-    });
-    let mut writer = BufWriter::new(get_target_output_writable(output_file.as_ref()));
+    let mut writer = BufWriter::new(get_target_output_writable(output_file));
     serde_json::to_writer(&mut writer, &result).unwrap();
 }
 
-pub fn output_files(results: &[(PathBuf, PathBuf, CovResult)], output_path: Option<&Path>) {
-    let output_file = output_path.map(|path| {
-        if path.is_dir() {
-            path.join("files")
-        } else {
-            path.to_path_buf()
-        }
-    });
-    let mut writer = BufWriter::new(get_target_output_writable(output_file.as_ref()));
+pub fn output_files(results: &[ResultTuple], output_file: Option<&Path>) {
+    let mut writer = BufWriter::new(get_target_output_writable(output_file));
     for (_, rel_path, _) in results {
         writeln!(writer, "{}", rel_path.display()).unwrap();
     }
 }
 
 pub fn output_html(
-    results: &[(PathBuf, PathBuf, CovResult)],
+    results: &[ResultTuple],
     output_dir: Option<&Path>,
     num_threads: usize,
     branch_enabled: bool,
@@ -570,7 +532,6 @@ pub fn output_html(
     } else {
         PathBuf::from("./html")
     };
-
     if output.exists() {
         if !output.is_dir() {
             eprintln!("{} is not a directory", output.to_str().unwrap());

src/path_rewriting.rs

@@ -8,6 +8,7 @@ use std::io;
 use std::path::{Component, Path, PathBuf};
 use walkdir::{DirEntry, WalkDir};
 
+use crate::ResultTuple;
 use crate::defs::*;
 use crate::filter::*;
 
@@ -235,7 +236,7 @@ pub fn rewrite_paths(
     to_keep_dirs: &[impl AsRef<str>],
     filter_option: Option<bool>,
     file_filter: crate::FileFilter,
-) -> Vec<(PathBuf, PathBuf, CovResult)> {
+) -> Vec<ResultTuple> {
     let to_ignore_globset = to_globset(to_ignore_dirs);
     let to_keep_globset = to_globset(to_keep_dirs);