From 6f32a1921ae0727eb69a7f204eed0f222e1bd54a Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sun, 27 Mar 2022 22:02:55 -0400 Subject: [PATCH] df: error on duplicate columns in --output arg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Print a usage error when duplicat column names are specified to the `--output` command-line argument. For example, $ df --output=source,source df: option --output: field ‘source’ used more than once Try 'df --help' for more information. --- src/uu/df/src/columns.rs | 60 ++++++++++++++++++++++++++-------------- src/uu/df/src/df.rs | 41 ++++++++++++++++++++++++--- tests/by-util/test_df.rs | 8 ++++++ 3 files changed, 85 insertions(+), 24 deletions(-) diff --git a/src/uu/df/src/columns.rs b/src/uu/df/src/columns.rs index 89dd3522077..91c52ed2e4f 100644 --- a/src/uu/df/src/columns.rs +++ b/src/uu/df/src/columns.rs @@ -55,19 +55,31 @@ pub(crate) enum Column { Capacity, } +/// An error while defining which columns to display in the output table. +#[derive(Debug)] +pub(crate) enum ColumnError { + /// If a column appears more than once in the `--output` argument. + MultipleColumns(String), +} + impl Column { /// Convert from command-line arguments to sequence of columns. /// /// The set of columns that will appear in the output table can be /// specified by command-line arguments. This function converts /// those arguments to a [`Vec`] of [`Column`] variants. - pub(crate) fn from_matches(matches: &ArgMatches) -> Vec { + /// + /// # Errors + /// + /// This function returns an error if a column is specified more + /// than once in the command-line argument. + pub(crate) fn from_matches(matches: &ArgMatches) -> Result, ColumnError> { match ( matches.is_present(OPT_PRINT_TYPE), matches.is_present(OPT_INODES), matches.occurrences_of(OPT_OUTPUT) > 0, ) { - (false, false, false) => vec![ + (false, false, false) => Ok(vec![ Self::Source, Self::Size, Self::Used, @@ -76,29 +88,37 @@ impl Column { Self::Capacity, Self::Pcent, Self::Target, - ], + ]), (false, false, true) => { - matches - .values_of(OPT_OUTPUT) - .unwrap() - .map(|s| { - // Unwrapping here should not panic because the - // command-line argument parsing library should be - // responsible for ensuring each comma-separated - // string is a valid column label. - Self::parse(s).unwrap() - }) - .collect() + // Unwrapping should not panic because in this arm of + // the `match` statement, we know that `OPT_OUTPUT` + // is non-empty. + let names = matches.values_of(OPT_OUTPUT).unwrap(); + let mut seen: Vec<&str> = vec![]; + let mut columns = vec![]; + for name in names { + if seen.contains(&name) { + return Err(ColumnError::MultipleColumns(name.to_string())); + } + seen.push(name); + // Unwrapping here should not panic because the + // command-line argument parsing library should be + // responsible for ensuring each comma-separated + // string is a valid column label. + let column = Self::parse(name).unwrap(); + columns.push(column); + } + Ok(columns) } - (false, true, false) => vec![ + (false, true, false) => Ok(vec![ Self::Source, Self::Itotal, Self::Iused, Self::Iavail, Self::Ipcent, Self::Target, - ], - (true, false, false) => vec![ + ]), + (true, false, false) => Ok(vec![ Self::Source, Self::Fstype, Self::Size, @@ -108,8 +128,8 @@ impl Column { Self::Capacity, Self::Pcent, Self::Target, - ], - (true, true, false) => vec![ + ]), + (true, true, false) => Ok(vec![ Self::Source, Self::Fstype, Self::Itotal, @@ -117,7 +137,7 @@ impl Column { Self::Iavail, Self::Ipcent, Self::Target, - ], + ]), // The command-line arguments -T and -i are each mutually // exclusive with --output, so the command-line argument // parser should reject those combinations before we get diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 6dd5ad43d20..0afb558710e 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -11,17 +11,19 @@ mod columns; mod filesystem; mod table; -use uucore::error::{UResult, USimpleError}; +use uucore::display::Quotable; +use uucore::error::{UError, UResult}; use uucore::format_usage; use uucore::fsext::{read_fs_list, MountInfo}; use clap::{crate_version, Arg, ArgMatches, Command}; +use std::error::Error; use std::fmt; use std::path::Path; use crate::blocks::{block_size_from_matches, BlockSize}; -use crate::columns::Column; +use crate::columns::{Column, ColumnError}; use crate::filesystem::Filesystem; use crate::table::{DisplayRow, Header, Row}; @@ -103,8 +105,12 @@ impl Default for Options { } } +#[derive(Debug)] enum OptionsError { InvalidBlockSize, + + /// An error getting the columns to display in the output table. + ColumnError(ColumnError), } impl fmt::Display for OptionsError { @@ -115,6 +121,11 @@ impl fmt::Display for OptionsError { // TODO This needs to vary based on whether `--block-size` // or `-B` were provided. Self::InvalidBlockSize => write!(f, "invalid --block-size argument"), + Self::ColumnError(ColumnError::MultipleColumns(s)) => write!( + f, + "option --output: field {} used more than once", + s.quote() + ), } } } @@ -131,7 +142,7 @@ impl Options { include: matches.values_of_lossy(OPT_TYPE), exclude: matches.values_of_lossy(OPT_EXCLUDE_TYPE), show_total: matches.is_present(OPT_TOTAL), - columns: Column::from_matches(matches), + columns: Column::from_matches(matches).map_err(OptionsError::ColumnError)?, }) } } @@ -273,6 +284,28 @@ where .collect() } +#[derive(Debug)] +enum DfError { + /// A problem while parsing command-line options. + OptionsError(OptionsError), +} + +impl Error for DfError {} + +impl UError for DfError { + fn usage(&self) -> bool { + matches!(self, Self::OptionsError(OptionsError::ColumnError(_))) + } +} + +impl fmt::Display for DfError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Self::OptionsError(e) => e.fmt(f), + } + } +} + #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uu_app().get_matches_from(args); @@ -284,7 +317,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } } - let opt = Options::from(&matches).map_err(|e| USimpleError::new(1, format!("{}", e)))?; + let opt = Options::from(&matches).map_err(DfError::OptionsError)?; // Get the list of filesystems to display in the output table. let filesystems: Vec = match matches.values_of(OPT_PATHS) { diff --git a/tests/by-util/test_df.rs b/tests/by-util/test_df.rs index 24277890d5e..69652bed043 100644 --- a/tests/by-util/test_df.rs +++ b/tests/by-util/test_df.rs @@ -264,3 +264,11 @@ fn test_output_file_specific_files() { ] ); } + +#[test] +fn test_output_field_no_more_than_once() { + new_ucmd!() + .arg("--output=target,source,target") + .fails() + .usage_error("option --output: field 'target' used more than once"); +}