ESS arguably should across-the-board qualify built function calls with package names

[malcook]

ESS is inconsistent as to whether built function calls to R's print function are qualified by the package, e.g. as base::print. viz:

$ grep 'print(' ~/.emacs.d/elpa/ess-20220201.816/*.el
/home/mec/.emacs.d/elpa/ess-20220201.816/ess-custom.el:(defcustom inferior-ess-r-objects-command "print(objects(pos=%d, all.names=TRUE), max=1e6)\n"
/home/mec/.emacs.d/elpa/ess-20220201.816/ess-custom.el:  "tryCatch(base::print(base::names(%s), max=1e6), error=function(e){})\n"
/home/mec/.emacs.d/elpa/ess-20220201.816/ess-inf.el:print(out, max=1e6) })\n\"."
/home/mec/.emacs.d/elpa/ess-20220201.816/ess-r-mode.el:          (ess-get-words-from-vector--foreground "print(rownames(available.packages()), max=1e6)\n")))
/home/mec/.emacs.d/elpa/ess-20220201.816/ess-r-mode.el:      (when-let ((M1 (ess-get-words-from-vector "local({out <- getCRANmirrors(local.only=TRUE); print(paste(out$Name,'[',out$URL,']', sep=''))})\n"))
/home/mec/.emacs.d/elpa/ess-20220201.816/ess-r-mode.el:  (unless (ess-boolean-command (format "print(requireNamespace('%s', quietly = TRUE))\n" pkg))
/home/mec/.emacs.d/elpa/ess-20220201.816/ess-r-mode.el:  (ess-get-words-from-vector--foreground "print(.packages(T), max=1e6)\n"))
/home/mec/.emacs.d/elpa/ess-20220201.816/ess-r-mode.el:                                        "print(installed.packages(fields=c(\"Title\"))[, c(\"Title\", \"Version\")]);"
/home/mec/.emacs.d/elpa/ess-20220201.816/ess-r-mode.el:       print(available.packages(fields=c(\"Title\"))[, c(\"Title\", \"Version\")])
/home/mec/.emacs.d/elpa/ess-20220201.816/ess-roxy.el:      (unless (ess-boolean-command (concat "print(suppressWarnings(require(" ess-roxy-package
/home/mec/.emacs.d/elpa/ess-20220201.816/ess-roxy.el:    (ess-command "print(suppressWarnings(require(tools, quietly=TRUE)))\n")
/home/mec/.emacs.d/elpa/ess-20220201.816/ess-r-package.el:               (format "print(.packages(%s), max = 1e6)\n"

Alas some other R packages redefine print in a way that breaks ESS!!!

For example, d3heatmap redefines it thusly:

> print
nonstandardGenericFunction for "print" defined from package "d3heatmap"

function (gadget) 
{
    standardGeneric("print")
}
<bytecode: 0x1d2e5a80>
<environment: 0x1d2dc8a8>
Methods may be defined for arguments: gadget
Use  showMethods(print)  for currently available ones.

This arguably is a bad idea and has been reported, but, regardless, ESS can protect against this by consistently calling base::print

One very notable breakage that occurs is repeated annoying occurrences of

Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  file-remote-p(nil localname)
  ess--derive-connection-path("/ssh:mec@hickory:/n/projects/mec/howto/emacs/orgmo..." nil)
  ess-synchronize-dirs()
  ess--idle-timer-function()
  apply(ess--idle-timer-function nil)
  timer-event-handler([t 0 1 0 repeat ess--idle-timer-function nil idle 0 nil])

which happens when ess--idle-timer-function's call to ess-synchronize-dirs in turn calls (ess-get-words-from-vector ess-getwd-command) which uses an unprotected print resulting in nil be returned as the incorrect presumed value of (getwd) which gets passed along to ess--derive-connection-path.

A fix is to prefix base:: to all built calls to print, and possibly more generally, to across-the-board qualify built function calls with package names.

This is possibly the root cause of #1180

[vspinu]

Yes. Have to do that indeed.

[lionel-]

I drafted a more robust way of dealing with this in #1170 (comment)

I'm now thinking that if we're going to introduce a breaking change, we should go all the way and evaluate commands in a child of base env rather than in a child of the current env. This way we improve robustness as objects and functions defined by the user can't interfere with commands. But this means functions in other packages (utils in particular) must be namespaced with ::, which will cause trouble for a while as people update their commands.

And #1170 (comment)

Another option is to create a clone of the utils package env that inherits from base. I think that's be a reasonable choice for the command environment, basically a private namespace with an import(utils) setting. Each command would be evaluated in a child so they can create bindings without littering the namespace. With both base and utils functions in scope, the breakages to existing commands should be limited.

However we need to find a solution for the ESS commands that use .ess.foo() utils. Those are currently attached to the search path and won't be in scope in such a command namespace. I'll keep thinking about this before drawing up a proposal.

It's easy to forget the namespace qualifiers, and it may make the code harder to read, so evaluating in a namespace seems better overall.