From bc9a5a31f3a8e397352b2ee9a3c14cd923971b99 Mon Sep 17 00:00:00 2001 From: "Trevor L. Davis" Date: Thu, 26 Mar 2026 22:38:53 -0700 Subject: [PATCH] feat: More classed errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Several new classed errors are now thrown, mirroring Python's `optparse` exception hierarchy: + `optparse_option_error` — invalid option definition in `make_option()` or `add_option()`. - `optparse_option_conflict_error` — duplicate flag in `OptionParser()` or `add_option()`. + `optparse_parse_error` — existing base class for all parse-time errors from `parse_args()`. - `optparse_bad_option_error` — unrecognized, misused, or argument-requiring option. * `optparse_ambiguous_option_error` — ambiguous abbreviated long flag. - `optparse_bad_positional_arguments_error` — wrong number of positional arguments supplied. * Also refactor to use internal (mockable) `is_interactive()` and improve some tests and error wordings. Co-Authored-By: Claude Sonnet 4.6 --- DESCRIPTION | 2 +- NEWS.md | 11 ++++ R/getopt.R | 24 +++++-- R/optparse.R | 104 +++++++++++++++++++++++------- README.Rrst | 15 +++++ man/add_make_option.Rd | 13 ++++ man/parse_args.Rd | 24 ++++--- tests/testthat/_snaps/optparse.md | 8 +-- tests/testthat/test-getopt.R | 12 ++++ tests/testthat/test-optparse.R | 57 ++++++++++++++-- 10 files changed, 223 insertions(+), 47 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 0715f40..f3b40b8 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -2,7 +2,7 @@ Encoding: UTF-8 Package: optparse Type: Package Title: Command Line Option Parser -Version: 1.8.0-5 +Version: 1.8.0-6 Authors@R: c(person("Trevor L.", "Davis", role=c("aut", "cre"), email="trevor.l.davis@gmail.com", comment = c(ORCID = "0000-0001-6341-4639")), diff --git a/NEWS.md b/NEWS.md index 754fc75..48e6e3a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -13,6 +13,17 @@ New features * `add_option()` (and `make_option()`) now support a `const` parameter (intended for new actions `"append_const"` and `"store_const"`). +* Several new classed errors are now thrown, mirroring Python's `optparse` exception hierarchy: + + + `optparse_option_error`: invalid option definition in `make_option()` or `add_option()`. + - `optparse_option_conflict_error`: duplicate flag in `OptionParser()` or `add_option()`. + + + `optparse_parse_error`: existing base class for all parse-time errors from `parse_args()`. + - `optparse_bad_option_error`: unrecognized, misused, or argument-requiring option. + * `optparse_ambiguous_option_error`: ambiguous abbreviated long flag. + - `optparse_bad_positional_arguments_error`: wrong number of positional arguments supplied. + - `optparse_missing_required_error`: a required option was not supplied. + * `add_option()` (and `make_option()`) now allow non-letter characters in short flags (e.g., `-1`) and at the beginning of long flags (e.g., `--1flag`), following Python's `optparse` convention. However long and short flags may not contain `=` or whitespace (or begin with hyphens). * `add_option()` (and `make_option()`) now support a `required` argument. diff --git a/R/getopt.R b/R/getopt.R index b50a040..ed7f178 100644 --- a/R/getopt.R +++ b/R/getopt.R @@ -73,7 +73,11 @@ getopt <- function( action %in% c("append_const", "count", "store_const", "store_false", "store_true") ) { - stop(paste0('long flag "', this_flag, '" accepts no arguments')) + bad_option_error_stop(paste0( + 'long flag "', + this_flag, + '" accepts no arguments' + )) } else if (action == "append") { result[[dest_name]] <- c(result[[dest_name]], this_argument) i <- i + 1L @@ -92,7 +96,7 @@ getopt <- function( i <- i + 1L next } else { - stop(paste0('"', optstring, '" is not a valid option, or does not support an argument')) + bad_option_error_stop(paste0('"', optstring, '" is not a valid option')) } long_name <- spec[rowmatch, COL_LONG_NAME] @@ -134,7 +138,13 @@ getopt <- function( } } if (action %in% c("store", "append")) { - stop(paste0('flag `', this_flag, '` requires an argument')) + flag_kind <- ifelse(is_long_flag(opt[i]), "long", "short") + bad_option_error_stop(paste0( + flag_kind, + ' flag "', + this_flag, + '" requires an argument' + )) } } } @@ -185,6 +195,8 @@ get_Rscript_filename <- function() { command_args <- function() commandArgs() +is_interactive <- function() interactive() + littler_script_path <- function() Sys.getenv("LITTLER_SCRIPT_PATH", unset = NA_character_) is_negative_number <- function(x) { @@ -245,17 +257,17 @@ get_rowmatch <- function(spec, long = NULL, short = NULL) { if (!is.null(long)) { rowmatch <- grep(long, spec[, COL_LONG_NAME], fixed = TRUE) if (length(rowmatch) == 0L) { - stop(paste0('long flag "', long, '" is invalid')) + bad_option_error_stop(paste0('long flag "', long, '" is invalid')) } else if (length(rowmatch) > 1L) { rowmatch <- which(long == spec[, COL_LONG_NAME]) if (length(rowmatch) == 0L) { - stop(paste0('long flag "', long, '" is ambiguous')) + ambiguous_option_error_stop(paste0('long flag "', long, '" is ambiguous')) } } } else { rowmatch <- grep(short, spec[, COL_SHORT_NAME], fixed = TRUE) if (length(rowmatch) == 0L) { - stop(paste0('short flag "', short, '" is invalid')) + bad_option_error_stop(paste0('short flag "', short, '" is invalid')) } } rowmatch diff --git a/R/optparse.R b/R/optparse.R index 2cb35c3..e92f46c 100644 --- a/R/optparse.R +++ b/R/optparse.R @@ -69,7 +69,7 @@ setValidity("OptionParser", function(object) { " (did you forget to set `add_help_option = FALSE` in `OptionParser()`?)" ) } - return(msg) + option_conflict_error_stop(msg) } short_flags <- na_omit(vapply(object@options, \(o) o@short_flag, character(1L))) if (anyDuplicated(short_flags)) { @@ -81,7 +81,7 @@ setValidity("OptionParser", function(object) { " (did you forget to set `add_help_option = FALSE` in `OptionParser()`?)" ) } - return(msg) + option_conflict_error_stop(msg) } TRUE }) @@ -205,6 +205,18 @@ OptionParser <- function( )) } +option_error_stop <- function(msg, call = sys.call(-1)) { + stop(errorCondition(msg, class = "optparse_option_error", call = call)) +} + +option_conflict_error_stop <- function(msg, call = sys.call(-1)) { + stop(errorCondition( + msg, + class = c("optparse_option_conflict_error", "optparse_option_error"), + call = call + )) +} + #' Functions to enable our OptionParser to recognize specific command line #' options. #' @@ -232,6 +244,13 @@ OptionParser <- function( #' @param callback_args `r ro_callback_args` #' @return Both [make_option()] and [add_option()] return instances of #' class `OptionParserOption`. +#' @section Errors: +#' The following classed errors may be thrown: +#' +#' - `optparse_option_error`: invalid option definition (bad flag string, +#' unrecognized action, etc.). +#' - `optparse_option_conflict_error`: duplicate flag detected when +#' adding an option to a parser. #' #' @seealso [parse_args()] [OptionParser()] #' @references Python's `optparse` library, which inspires this package, @@ -280,28 +299,28 @@ make_option <- function( short_flag <- NA_character_ } else { if (nchar(short_flag) > 2) { - stop(paste( + option_error_stop(paste( "Short flag", short_flag, "must only be a '-' and a single non-dash character" )) } if (grepl("[[:space:]]", short_flag)) { - stop(paste("Short flag", short_flag, "must not contain whitespace")) + option_error_stop(paste("Short flag", short_flag, "must not contain whitespace")) } if (grepl("=", short_flag, fixed = TRUE)) { - stop(paste("Short flag", short_flag, "must not contain '='")) + option_error_stop(paste("Short flag", short_flag, "must not contain '='")) } } long_flag <- opt_str[grepl("^--[^-]", opt_str)] if (length(long_flag) == 0) { - stop("We require a long flag option") + option_error_stop("We require a long flag option") } if (grepl("=", long_flag, fixed = TRUE)) { - stop(paste("Long flag", long_flag, "must not contain '='")) + option_error_stop(paste("Long flag", long_flag, "must not contain '='")) } if (grepl("[[:space:]]", long_flag)) { - stop(paste("Long flag", long_flag, "must not contain whitespace")) + option_error_stop(paste("Long flag", long_flag, "must not contain whitespace")) } # type @@ -564,14 +583,17 @@ as_string <- function(default) { #' positional arguments. For backward compatibility, if and only if #' `positional_arguments` is `FALSE`, returns a list containing #' option values. -#' @section Acknowledgement: -#' A big thanks to Steve Lianoglou for a bug report and patch; -#' Juan Carlos \enc{Borrás}{Borras} for a bug report; -#' Jim Nikelski for a bug report and patch; -#' Ino de Brujin and Benjamin Tyner for a bug report; -#' Jonas Zimmermann for bug report; Miroslav Posta for bug reports; -#' Stefan Seemayer for bug report and patch; -#' Kirill \enc{Müller}{Muller} for patches; Steve Humburg for patch. +#' @section Errors: +#' The following classed errors may be thrown: +#' +#' - `optparse_parse_error`: base class for all parse-time errors. +#' - `optparse_bad_option_error`: unrecognized, misused, or +#' argument-requiring option. +#' - `optparse_ambiguous_option_error`: ambiguous abbreviated long +#' flag. +#' - `optparse_bad_positional_arguments_error`: wrong number of +#' positional arguments supplied. +#' - `optparse_missing_required_error`: a required option was not supplied. #' #' @seealso [OptionParser()] [print_help()] #' @references Python's `optparse` library, which inspired this package, @@ -634,13 +656,51 @@ quieter_error_handler <- function(e) { quit('no', status = 1, runLast = FALSE) } +bad_option_error_stop <- function(msg) { + stop(errorCondition( + msg, + class = c("optparse_bad_option_error", "optparse_parse_error"), + call = NULL + )) +} + +ambiguous_option_error_stop <- function(msg) { + stop(errorCondition( + msg, + class = c( + "optparse_ambiguous_option_error", + "optparse_bad_option_error", + "optparse_parse_error" + ), + call = NULL + )) +} + +missing_required_error_stop <- function(msg) { + stop(errorCondition( + msg, + class = c("optparse_missing_required_error", "optparse_parse_error"), + call = NULL + )) +} + +bad_positional_arguments_error_stop <- function(msg) { + stop(errorCondition( + msg, + class = c("optparse_bad_positional_arguments_error", "optparse_parse_error"), + call = NULL + )) +} + pa_stop <- function(object, e) { + extra_classes <- setdiff(class(e), c("simpleError", "error", "condition")) + classes <- unique(c(extra_classes, "optparse_parse_error")) cnd <- errorCondition( e$message, call = "optparse::parse_args_helper()", - class = "optparse_parse_error" + class = classes ) - if (interactive()) { + if (is_interactive()) { stop(cnd) } else { signalCondition(cnd) @@ -682,7 +742,7 @@ parse_args_helper <- function( if (any(grepl("^help$", names(options_list)))) { if (options_list[["help"]] && print_help_and_exit) { print_help(object) - if (interactive()) { + if (is_interactive()) { stop("help requested") } else { quit(status = 0) @@ -697,21 +757,21 @@ parse_args_helper <- function( } } if (length(missing_required)) { - stop(paste( + missing_required_error_stop(paste( "the following arguments are required:", paste(missing_required, collapse = ", ") )) } if (length(arguments_positional) < min(positional_arguments)) { - stop(sprintf( + bad_positional_arguments_error_stop(sprintf( "required at least %g positional arguments, got %g", min(positional_arguments), length(arguments_positional) )) } if (length(arguments_positional) > max(positional_arguments)) { - stop(sprintf( + bad_positional_arguments_error_stop(sprintf( "required at most %g positional arguments, got %g", max(positional_arguments), length(arguments_positional) diff --git a/README.Rrst b/README.Rrst index a588ec8..17eb90d 100644 --- a/README.Rrst +++ b/README.Rrst @@ -107,3 +107,18 @@ The function ``parse_args2`` wraps ``parse_args`` while setting ``positional_arg parse_args2(parser, args = c("-vc", "25", "75", "22")) .. .. + +acknowledgements +---------------- + +A big thanks to: + +* Steve Lianoglou for a bug report and patch +* Juan Carlos Borrás for a bug report +* Jim Nikelski for a bug report and patch +* Ino de Brujin and Benjamin Tyner for a bug report +* Jonas Zimmermann for a bug report +* Miroslav Posta for bug reports +* Stefan Seemayer for a bug report and patch +* Kirill Müller for patches +* Steve Humburg for a patch diff --git a/man/add_make_option.Rd b/man/add_make_option.Rd index ebc7704..3da0a52 100644 --- a/man/add_make_option.Rd +++ b/man/add_make_option.Rd @@ -92,6 +92,19 @@ whereas \code{\link[=make_option]{make_option()}} is used to create a list of \code{option_list} argument of the \code{OptionParser} function to create a new \code{OptionParser} instance. } +\section{Errors}{ + +The following classed errors may be thrown: +\itemize{ +\item \code{optparse_option_error}: invalid option definition (bad flag string, +unrecognized action, etc.). +\itemize{ +\item \code{optparse_option_conflict_error}: duplicate flag detected when +adding an option to a parser. +} +} +} + \examples{ make_option("--longflag") diff --git a/man/parse_args.Rd b/man/parse_args.Rd index 0cc94f4..6041630 100644 --- a/man/parse_args.Rd +++ b/man/parse_args.Rd @@ -57,15 +57,23 @@ instance for guidance. \code{\link[=parse_args2]{parse_args2()}} is a wrapper to setting the options \code{positional_arguments} and \code{convert_hyphens_to_underscores} to \code{TRUE}. } -\section{Acknowledgement}{ +\section{Errors}{ -A big thanks to Steve Lianoglou for a bug report and patch; -Juan Carlos \enc{Borrás}{Borras} for a bug report; -Jim Nikelski for a bug report and patch; -Ino de Brujin and Benjamin Tyner for a bug report; -Jonas Zimmermann for bug report; Miroslav Posta for bug reports; -Stefan Seemayer for bug report and patch; -Kirill \enc{Müller}{Muller} for patches; Steve Humburg for patch. +The following classed errors may be thrown: +\itemize{ +\item \code{optparse_parse_error}: base class for all parse-time errors. +\itemize{ +\item \code{optparse_bad_option_error}: unrecognized, misused, or +argument-requiring option. +\itemize{ +\item \code{optparse_ambiguous_option_error}: ambiguous abbreviated long +flag. +} +\item \code{optparse_bad_positional_arguments_error}: wrong number of +positional arguments supplied. +\item \code{optparse_missing_required_error}: a required option was not supplied. +} +} } \examples{ diff --git a/tests/testthat/_snaps/optparse.md b/tests/testthat/_snaps/optparse.md index 4ba57c0..d7fd49d 100644 --- a/tests/testthat/_snaps/optparse.md +++ b/tests/testthat/_snaps/optparse.md @@ -110,7 +110,7 @@ parse_args(parser, c("file.txt")) Condition Error: - ! "file.txt" is not a valid option, or does not support an argument + ! "file.txt" is not a valid option --- @@ -118,15 +118,15 @@ parse_args(parser, c("--verbose", "file.txt")) Condition Error: - ! "file.txt" is not a valid option, or does not support an argument + ! "file.txt" is not a valid option # Use h option for non-help Code OptionParser(usage = "\\%prog [options] file", option_list = option_list_neg) Condition - Error in `validObject()`: - ! invalid class "OptionParser" object: duplicate short flag: -h (did you forget to set `add_help_option = FALSE` in `OptionParser()`?) + Error in `validityMethod()`: + ! duplicate short flag: -h (did you forget to set `add_help_option = FALSE` in `OptionParser()`?) # no-argument actions reject --flag=value syntax diff --git a/tests/testthat/test-getopt.R b/tests/testthat/test-getopt.R index 234f0cf..df605e2 100644 --- a/tests/testthat/test-getopt.R +++ b/tests/testthat/test-getopt.R @@ -1,3 +1,10 @@ +test_that("flag that requires an argument raises optparse_bad_option_error", { + parser <- OptionParser() + parser <- add_option(parser, c("-n", "--number"), type = "integer") + expect_error(parse_args(parser, c("--number")), class = "optparse_bad_option_error") + expect_error(parse_args(parser, c("-n")), class = "optparse_bad_option_error") +}) + test_that("negative-number-looking bundle is passed as argument or expanded to short flags", { parser <- OptionParser() parser <- add_option(parser, c("-m", "--mean"), type = "double") @@ -8,8 +15,10 @@ test_that("negative-number-looking bundle is passed as argument or expanded to s # long flag with = preceding negative number bundle: = already consumed value, # so prev_takes_argument() falls to else (line 215) and -3.14 is expanded → error expect_snapshot(error = TRUE, parse_args(parser, c("--mean=5", "-3.14"))) + expect_error(parse_args(parser, c("--mean=5", "-3.14")), class = "optparse_bad_option_error") # standalone: expanded to -1, -2 which are invalid short flags expect_snapshot(error = TRUE, parse_args(parser, c("-12"))) + expect_error(parse_args(parser, c("-12")), class = "optparse_bad_option_error") }) test_that("type coercion warns when value cannot be converted to expected type", { @@ -27,6 +36,9 @@ test_that("abbreviated long flag matching two options raises ambiguous error", { parser <- add_option(parser, "--verbose", action = "store_true") parser <- add_option(parser, "--verbosity", type = "integer") expect_snapshot(error = TRUE, parse_args(parser, c("--verb"))) + expect_error(parse_args(parser, c("--verb")), class = "optparse_ambiguous_option_error") + expect_error(parse_args(parser, c("--verb")), class = "optparse_bad_option_error") + expect_error(parse_args(parser, c("--verb")), class = "optparse_parse_error") }) test_that("get_Rscript_filename() ignores --file= after --args", { diff --git a/tests/testthat/test-optparse.R b/tests/testthat/test-optparse.R index 795305c..c2a9c20 100644 --- a/tests/testthat/test-optparse.R +++ b/tests/testthat/test-optparse.R @@ -67,6 +67,8 @@ test_that("`make_option()` works as expected", { expect_equal(make_option("--filename")@type, "character") expect_snapshot(error = TRUE, make_option("badflag")) expect_error(make_option("-cd"), "must only be") + expect_error(make_option("badflag"), class = "optparse_option_error") + expect_error(make_option("-cd"), class = "optparse_option_error") expect_equal(make_option(c("-1", "--one"))@short_flag, "-1") expect_equal(make_option("--1flag")@long_flag, "--1flag") expect_equal(make_option("--add-numbers")@long_flag, "--add-numbers") @@ -91,6 +93,7 @@ test_that("`required` argument works as expected", { parser <- OptionParser() parser <- add_option(parser, "--foo", required = TRUE) expect_snapshot(error = TRUE, parse_args(parser, args = character(0))) + expect_error(parse_args(parser, args = character(0)), class = "optparse_missing_required_error") expect_equal( parse_args(parser, args = "--foo=bar")[["foo"]], "bar" @@ -177,6 +180,10 @@ test_that("`parse_args()` works as expected", { error = TRUE, parse_args(parser, args = c("-add-numbers", "example.txt"), positional_arguments = FALSE) ) + expect_error( + parse_args(parser, args = c("-add-numbers", "example.txt"), positional_arguments = FALSE), + class = "optparse_bad_option_error" + ) expect_error(parse_args( parser, args = c("-add-numbers", "example.txt"), @@ -205,10 +212,18 @@ test_that("`parse_args()` works as expected", { error = TRUE, parse_args(parser, args = c("example.txt"), positional_arguments = c(2, Inf)) ) + expect_error( + parse_args(parser, args = c("example.txt"), positional_arguments = c(2, Inf)), + class = "optparse_bad_positional_arguments_error" + ) expect_snapshot( error = TRUE, parse_args(parser, args = c("example.txt"), positional_arguments = 2) ) + expect_error( + parse_args(parser, args = c("example.txt"), positional_arguments = 2), + class = "optparse_bad_positional_arguments_error" + ) expect_snapshot( error = TRUE, parse_args(parser, args = c("example.txt"), positional_arguments = "any") @@ -217,14 +232,35 @@ test_that("`parse_args()` works as expected", { error = TRUE, parse_args(parser, args = c("example.txt"), positional_arguments = 1:3) ) +}) - if (interactive()) { - expect_snapshot(error = TRUE, capture.output(parse_args(parser, args = c("--help")))) - expect_snapshot( - error = TRUE, - capture.output(parse_args(parser, args = c("--help"), positional_arguments = c(1, 2))) +test_that("--help raises an error in interactive mode", { + parser <- OptionParser( + usage = "%prog [options] file", + option_list = list( + make_option(c("-n", "--add-numbers"), action = "store_true", default = FALSE) ) - } + ) + local_mocked_bindings(is_interactive = function() TRUE) + capture.output(expect_error(parse_args(parser, args = c("--help")), "help requested")) + capture.output(expect_error( + parse_args(parser, args = c("--help"), positional_arguments = c(1, 2)), + "help requested" + )) +}) + +test_that("parse errors raise classed errors in interactive and non-interactive mode", { + parser <- OptionParser() + local_mocked_bindings(is_interactive = function() TRUE) + expect_error( + parse_args(parser, c("file.txt")), + class = "optparse_bad_option_error" + ) + local_mocked_bindings(is_interactive = function() FALSE) + capture.output( + expect_error(parse_args(parser, c("file.txt")), class = "optparse_bad_option_error"), + type = "message" + ) }) test_that("bare -- separator treats everything after it as positional arguments", { @@ -568,6 +604,14 @@ test_that("Use h option for non-help", { error = TRUE, OptionParser(usage = "\\%prog [options] file", option_list = option_list_neg) ) + expect_error( + OptionParser(usage = "\\%prog [options] file", option_list = option_list_neg), + class = "optparse_option_conflict_error" + ) + expect_error( + OptionParser(usage = "\\%prog [options] file", option_list = option_list_neg), + class = "optparse_option_error" + ) option_list_neg <- list(make_option(c("-h", "--mean"), default = 0.0)) parser <- OptionParser( @@ -591,6 +635,7 @@ test_that("no-argument actions reject --flag=value syntax", { expect_snapshot(error = TRUE, parse_args(parser, "--mode=1")) expect_snapshot(error = TRUE, parse_args(parser, "--tag=1")) expect_snapshot(error = TRUE, parse_args(parser, "--count=1")) + expect_error(parse_args(parser, "--verbose=1"), class = "optparse_bad_option_error") }) test_that("store_const action works", {