Why does base::detach validate name inputs the way it does?

Question

After working my way thru Detach several packages at once , I now wonder if there is a reason the following validation code in base::detach is the way it is, or if there's a bug?

(the function itself is detach(name, pos = 2L, unload = FALSE, character.only = FALSE, force = FALSE) )

if (!missing(name)) {
        if (!character.only) 
            name <- substitute(name)
        pos <- if (is.numeric(name)) 
            name
        else {
            if (!is.character(name)) 
                name <- deparse(name)
            match(name, search())
        }
        if (is.na(pos)) 
            stop("invalid 'name' argument")

It seems to me that, whether or not the "character.only" argument is set, it would be better to test is.character(name) prior to risking running substitute() on the name value. The documentation does not explicitly say that you MUST set character.only=TRUE when name is a string.
So, before I go off and submit a bug/enhancement request, is there a solid reason for the validation to be the way it is?

Edit: to comment on mnel's response, here's my test example.

Rgames> detlist<-c('Hmisc','survival','splines')
Rgames> library(Hmisc)
 Rgames> debug(base::detach)
Rgames> base::detach(detlist[1])

# skipped the startup stuff.

Browse[2]> 
debug: if (!character.only) name <- substitute(name)
Browse[2]> name
[1] "Hmisc"
Browse[2]> 
debug: name <- substitute(name)
Browse[2]> 
debug: pos <- if (is.numeric(name)) name else {
    if (!is.character(name)) 
        name <- deparse(name)
    match(name, search())
}
Browse[2]> name
detlist[1]
Browse[2]> 
debug: if (!is.character(name)) name <- deparse(name)
Browse[2]> 
debug: name <- deparse(name)
Browse[2]> deparse(name)
[1] "detlist[1]"

So you see the problem. My input variable is a valid name, but failure to set character.only to TRUE causes an undesired deparse to take place, and match(name, search()) fails for obvious reasons.
It just seems to me that it would be easier for the user if is.character were checked first, albeit inside a tryCatch to deal with the situation mnel describes. Two reasons: 1) it's more "user friendly" if detach doesn't even need a character.only argument, and 2) the documentation at present doesn't warn that the user must set character.only==TRUE when the name argument is an object containing a character string (but not, I believe if it's a just plain string. Either detach(package:Hmisc) or detach("package:Hmisc") work, but as my example shows, not a reference to the same string).

mnel · Accepted Answer · 2013-07-12T04:59:47.090

It is a matter of not risking evaluating the name argument unless you know it is character only.

Note that help('detach') describes the name argument as

This can be an unquoted name or a character string but not a character vector. If a number is supplied this is taken as pos.

is.character will evaluate its argument whereas substitute will not. If name is not a character string , it will be an unquoted string (such as package:stats). If you were to test is.character(x = package:stats), this will try to evaluate package:stats (i.e a call to `:`(package,stats). This would not be good (and will return an error or something nonsensical, depending on whether you have values for package and stats available on your search path).

using character.only, means that substitute will be called (which won't evaluate the argument), but will return an object of language type (if the original were an unquoted name) or a character string (if it were a character string originally). Whatever the outcome, name can be deparsed to create the required character string.

You can trace what is happening by creating a function with a well placed call to browser

eg

d <- function (name, pos = 2L, unload = FALSE, character.only = FALSE, 
          force = FALSE) {

  if (!missing(name)) {
    browser()
    if (!character.only) 
      name <- substitute(name)
    pos <- if (is.numeric(name)) 
      name
    else {
      if (!is.character(name)) 
        name <- deparse(name)
      match(name, search())
    }
    if (is.na(pos)) 
      stop("invalid 'name' argument")

}
  return(list(name, pos))
}

# called using an unquoted string.

d(package:stats, pos = 2L)
# Called from: d(package:stats, pos = 2L)
# Browse[1]> n
# debug at #7: if (!character.only) name <- substitute(name)
# Browse[2]> str(name)
# Error in str(name) : object 'package' not found
# Browse[2]> n
# debug at #7: name <- substitute(name)
# Browse[2]> n
# debug at #9: pos <- if (is.numeric(name)) name else {
# if (!is.character(name)) 
#   name <- deparse(name)
# match(name, search())
# }
# Browse[2]> str(name)
# language package:stats
# Browse[2]> n
# debug at #12: if (!is.character(name)) name <- deparse(name)
# Browse[2]> n
# debug at #12: name <- deparse(name)
# Browse[2]> n
# debug at #14: match(name, search())
# Browse[2]> n
# debug at #16: if (is.na(pos)) stop("invalid 'name' argument")
# Browse[2]> n
# debug at #16: NULL
# Browse[2]> str(name)
# chr "package:stats
# Browse[2]> Q

d('package:stats', pos = 2L)
# Called from: d("package:stats", pos = 2L)
# Browse[1]> n
# debug at #7: if (!character.only) name <- substitute(name)
# Browse[2]> n
# debug at #7: name <- substitute(name)
# Browse[2]> n
# debug at #9: pos <- if (is.numeric(name)) name else {
# if (!is.character(name)) 
#   name <- deparse(name)
# match(name, search())
# }
# Browse[2]> str(name)
# chr "package:stats"
# Browse[2]> Q

Edit in light of edited question.

Perhaps the way name is described in the help file should be consistent with the definition of 'packageinrequire&library(which have similar methods of dealing withnameandcharacter.string` style arguments

i.e

the name of a package, given as a name or literal character string, or a character string, depending on whether character.only is FALSE (default) or TRUE).

The current definition in help('detach') uses character string to imply a literal character string containing the name of the item on the search path you wish to detach and character vector to define a character vector which contains a character string that is the name of the item on the search path you wish to detach.

I changing the definition in help('detach') to

This can be an unquoted name or a literal character string. If character.only == TRUE then a character vector of length 1 may be supplied. If a number is supplied this is taken as pos.

or similar may be helpful.

Hi, please see my edited question which I hope addresses your explanation as well. — Carl Witthoft, Jul 11 '13 at 13:10
@CarlWitthoft -- see my edited answer. Perhaps a documentation patch could be supplied to make it consistent with `?require` — mnel, Jul 12 '13 at 05:00
`character.only` is misnamed, as I mentioned in my reply to the OP's linked question. It should really be `reject.names` or something similar. We're kind of stuck with it now, though. — Hong Ooi, Jul 12 '13 at 06:54
Sounds good to me. I agree a slight mod to the documentation page would be helpful. — Carl Witthoft, Jul 12 '13 at 11:43

Why does base::detach validate name inputs the way it does?

1 Answers1