From 507272421a3c32285c52505a9103dd9534fa558b Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Wed, 6 Dec 2023 13:44:13 +0800 Subject: [PATCH] Reduce duplication for maintaining docs of undesirable functions/operators (#2373) * reduce duplication for maintaining docs of undesirable functions/operators * shorter object names * link operators * fix * capture package too --------- Co-authored-by: AshesITR --- R/zzz.R | 101 ++++++++++++++------------- man/default_undesirable_functions.Rd | 68 +++++++----------- 2 files changed, 78 insertions(+), 91 deletions(-) diff --git a/R/zzz.R b/R/zzz.R index fb4f20f96..951e1752a 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -47,53 +47,13 @@ default_linters <- modify_defaults( #' There is a list for the default elements and another that contains all available elements. #' Use [modify_defaults()] to produce a custom list. #' -#' @details -#' The following functions are sometimes regarded as undesirable: -#' -#' * [attach()] modifies the global search path. Use roxygen2's @importFrom statement in packages, or `::` in scripts. -#' * [browser()] pauses execution when run and is likely a leftover from debugging. It should be removed. -#' * [debug()] traps a function and causes execution to pause when that function is run. It should be removed. -#' * [debugcall()] works similarly to [debug()], causing execution to pause. It should be removed. -#' * [debugonce()] is only useful for interactive debugging. It should be removed. -#' * [detach()] modifies the global search path. Detaching environments from the search path is rarely necessary in -#' production code. -#' * [ifelse()] isn't type stable. Use an `if`/`else` block for scalar logic, or use -#' `dplyr::if_else()`/`data.table::fifelse()` for type stable vectorized logic. -#' * [.libPaths()] permanently modifies the library location. Use [withr::with_libpaths()] for a temporary change -#' instead. -#' * [library()] modifies the global search path. Use roxygen2's @importFrom statement in packages, or `::` in scripts. -#' * [loadNamespace()] doesn't provide an easy way to signal failures. Use the return value of [requireNamespace()] -#' instead. -#' * [mapply()] isn't type stable. Use [Map()] to guarantee a list is returned and simplify accordingly. -#' * [options()] permanently modifies the session options. Use [withr::with_options()] for a temporary change instead. -#' * [par()] permanently modifies the graphics device parameters. Use [withr::with_par()] for a temporary change -#' instead. -#' * [require()] modifies the global search path. Use roxygen2's @importFrom statement in packages, and [library()] -#' or `::` in scripts. -#' * [sapply()] isn't type stable. Use [vapply()] with an appropriate `FUN.VALUE=` argument to obtain type stable -#' simplification. -#' * [setwd()] modifies the global working directory. Use [withr::with_dir()] for a temporary change instead. -#' * [sink()] permanently redirects output. Use [withr::with_sink()] for a temporary redirection instead. -#' * [source()] loads code into the global environment unless `local = TRUE` is used, which can cause unexpected -#' behavior. -#' * [substring()] should be replaced by [substr()] with appropriate `stop=` value. -#' * [Sys.setenv()] permanently modifies the global environment variables. Use [withr::with_envvar()] for a temporary -#' change instead. -#' * [Sys.setlocale()] permanently modifies the session locale. Use [withr::with_locale()] for a temporary change -#' instead. -#' * [trace()] traps a function and causes execution of arbitrary code when that function is run. It should be removed. -#' * [undebug()] is only useful for interactive debugging with [debug()]. It should be removed. -#' * [untrace()] is only useful for interactive debugging with [trace()]. It should be removed. -#' -#' The following operators are sometimes regarded as undesirable: -#' -#' * \code{\link[base:ns-dblcolon]{:::}} accesses non-exported functions inside packages. Code relying on these is -#' likely to break in future versions of the package because the functions are not part of the public interface and -#' may be changed or removed by the maintainers without notice. -#' Use public functions via `::` instead. -#' * [`<<-`][base::assignOps] and `->>` assign outside the current environment in a way that can be hard to reason -#' about. Prefer fully-encapsulated functions wherever possible, or, if necessary, assign to a specific environment -#' with [assign()]. Recall that you can create an environment at the desired scope with [new.env()]. +#' @evalRd c( +#' "\\details{", +#' rd_undesirable_functions(), +#' "", +#' rd_undesirable_operators(), +#' "}" +#' ) #' #' @format A named list of character strings. #' @rdname default_undesirable_functions @@ -156,7 +116,7 @@ all_undesirable_functions <- modify_defaults( "which can cause hard-to-predict behavior" ), structure = - "Use class<-, names<-, and attr<- to set attributes", + "Use `class<-`, `names<-`, and `attr<-` to set attributes", substring = "use substr() with appropriate `stop=` value.", Sys.setenv = @@ -207,6 +167,28 @@ default_undesirable_functions <- all_undesirable_functions[names(all_undesirable NULL )] +rd_auto_link <- function(x) { + x <- unlist(x) + x <- gsub("([a-zA-Z0-9.]+)::([a-zA-Z0-9._]+)\\(\\)", "\\\\code{\\\\link[\\1:\\2]{\\1::\\2()}}", x) + x <- gsub("([^:a-zA-Z0-9._])([a-zA-Z0-9._]+)\\(\\)", "\\1\\\\code{\\\\link[=\\2]{\\2()}}", x) + x <- gsub("`([^`]+)`", "\\\\code{\\1}", x) + x +} + +rd_undesirable_functions <- function() { + alternatives <- rd_auto_link(default_undesirable_functions) + + c( + "The following functions are sometimes regarded as undesirable:", + "\\itemize{", + sprintf( + "\\item \\code{\\link[=%1$s]{%1$s()}} As an alternative, %2$s.", + names(default_undesirable_functions), alternatives + ), + "}" + ) +} + #' @rdname default_undesirable_functions #' @format NULL #' @export @@ -215,7 +197,7 @@ all_undesirable_operators <- modify_defaults( ":::" = paste( "It accesses non-exported functions inside packages. Code relying on these is likely to break in", "future versions of the package because the functions are not part of the public interface and may be", - "changed or removed by the maintainers without notice. Use public functions via :: instead." + "changed or removed by the maintainers without notice. Use public functions via `::` instead." ), "<<-" = paste( "It assigns outside the current environment in a way that can be hard to reason about.", @@ -241,6 +223,27 @@ default_undesirable_operators <- all_undesirable_operators[names(all_undesirable NULL )] +rd_undesirable_operators <- function() { + op_link_map <- c( + `:::` = "\\link[base:ns-dblcolon]{:::}", + `<<-` = "\\link[base:assignOps]{<<-}", + `->>` = "\\link[base:assignOps]{<<-}" + ) + op <- names(default_undesirable_operators) + + alternatives <- rd_auto_link(default_undesirable_operators) + + c( + "The following operators are sometimes regarded as undesirable:", + "\\itemize{", + sprintf( + "\\item \\code{%1$s} As an alternative, %2$s", + op_link_map[op], alternatives + ), + "}" + ) +} + #' Default lintr settings #' #' @description diff --git a/man/default_undesirable_functions.Rd b/man/default_undesirable_functions.Rd index 47ad7a857..7562e02c8 100644 --- a/man/default_undesirable_functions.Rd +++ b/man/default_undesirable_functions.Rd @@ -24,54 +24,38 @@ Lists of function names and operators for \code{\link[=undesirable_function_lint There is a list for the default elements and another that contains all available elements. Use \code{\link[=modify_defaults]{modify_defaults()}} to produce a custom list. } +\keyword{datasets} \details{ The following functions are sometimes regarded as undesirable: \itemize{ -\item \code{\link[=attach]{attach()}} modifies the global search path. Use roxygen2's @importFrom statement in packages, or \code{::} in scripts. -\item \code{\link[=browser]{browser()}} pauses execution when run and is likely a leftover from debugging. It should be removed. -\item \code{\link[=debug]{debug()}} traps a function and causes execution to pause when that function is run. It should be removed. -\item \code{\link[=debugcall]{debugcall()}} works similarly to \code{\link[=debug]{debug()}}, causing execution to pause. It should be removed. -\item \code{\link[=debugonce]{debugonce()}} is only useful for interactive debugging. It should be removed. -\item \code{\link[=detach]{detach()}} modifies the global search path. Detaching environments from the search path is rarely necessary in -production code. -\item \code{\link[=ifelse]{ifelse()}} isn't type stable. Use an \code{if}/\verb{else} block for scalar logic, or use -\code{dplyr::if_else()}/\code{data.table::fifelse()} for type stable vectorized logic. -\item \code{\link[=.libPaths]{.libPaths()}} permanently modifies the library location. Use \code{\link[withr:with_libpaths]{withr::with_libpaths()}} for a temporary change -instead. -\item \code{\link[=library]{library()}} modifies the global search path. Use roxygen2's @importFrom statement in packages, or \code{::} in scripts. -\item \code{\link[=loadNamespace]{loadNamespace()}} doesn't provide an easy way to signal failures. Use the return value of \code{\link[=requireNamespace]{requireNamespace()}} -instead. -\item \code{\link[=mapply]{mapply()}} isn't type stable. Use \code{\link[=Map]{Map()}} to guarantee a list is returned and simplify accordingly. -\item \code{\link[=options]{options()}} permanently modifies the session options. Use \code{\link[withr:with_options]{withr::with_options()}} for a temporary change instead. -\item \code{\link[=par]{par()}} permanently modifies the graphics device parameters. Use \code{\link[withr:with_par]{withr::with_par()}} for a temporary change -instead. -\item \code{\link[=require]{require()}} modifies the global search path. Use roxygen2's @importFrom statement in packages, and \code{\link[=library]{library()}} -or \code{::} in scripts. -\item \code{\link[=sapply]{sapply()}} isn't type stable. Use \code{\link[=vapply]{vapply()}} with an appropriate \verb{FUN.VALUE=} argument to obtain type stable -simplification. -\item \code{\link[=setwd]{setwd()}} modifies the global working directory. Use \code{\link[withr:with_dir]{withr::with_dir()}} for a temporary change instead. -\item \code{\link[=sink]{sink()}} permanently redirects output. Use \code{\link[withr:with_sink]{withr::with_sink()}} for a temporary redirection instead. -\item \code{\link[=source]{source()}} loads code into the global environment unless \code{local = TRUE} is used, which can cause unexpected -behavior. -\item \code{\link[=substring]{substring()}} should be replaced by \code{\link[=substr]{substr()}} with appropriate \verb{stop=} value. -\item \code{\link[=Sys.setenv]{Sys.setenv()}} permanently modifies the global environment variables. Use \code{\link[withr:with_envvar]{withr::with_envvar()}} for a temporary -change instead. -\item \code{\link[=Sys.setlocale]{Sys.setlocale()}} permanently modifies the session locale. Use \code{\link[withr:with_locale]{withr::with_locale()}} for a temporary change -instead. -\item \code{\link[=trace]{trace()}} traps a function and causes execution of arbitrary code when that function is run. It should be removed. -\item \code{\link[=undebug]{undebug()}} is only useful for interactive debugging with \code{\link[=debug]{debug()}}. It should be removed. -\item \code{\link[=untrace]{untrace()}} is only useful for interactive debugging with \code{\link[=trace]{trace()}}. It should be removed. +\item \code{\link[=attach]{attach()}} As an alternative, use roxygen2's @importFrom statement in packages, or \code{::} in scripts. \code{\link[=attach]{attach()}} modifies the global search path. +\item \code{\link[=browser]{browser()}} As an alternative, remove this likely leftover from debugging. It pauses execution when run. +\item \code{\link[=debug]{debug()}} As an alternative, remove this likely leftover from debugging. It traps a function and causes execution to pause when that function is run. +\item \code{\link[=debugcall]{debugcall()}} As an alternative, remove this likely leftover from debugging. It traps a function and causes execution to pause when that function is run. +\item \code{\link[=debugonce]{debugonce()}} As an alternative, remove this likely leftover from debugging. It traps a function and causes execution to pause when that function is run. +\item \code{\link[=detach]{detach()}} As an alternative, avoid modifying the global search path. Detaching environments from the search path is rarely necessary in production code. +\item \code{\link[=.libPaths]{.libPaths()}} As an alternative, use \code{\link[withr:with_libpaths]{withr::with_libpaths()}} for a temporary change instead of permanently modifying the library location. +\item \code{\link[=library]{library()}} As an alternative, use roxygen2's @importFrom statement in packages and \code{::} in scripts, instead of modifying the global search path. +\item \code{\link[=mapply]{mapply()}} As an alternative, use \code{\link[=Map]{Map()}} to guarantee a list is returned and simplify accordingly. +\item \code{\link[=options]{options()}} As an alternative, use \code{\link[withr:with_options]{withr::with_options()}} for a temporary change instead of permanently modifying the session options. +\item \code{\link[=par]{par()}} As an alternative, use \code{\link[withr:with_par]{withr::with_par()}} for a temporary change instead of permanently modifying the graphics device parameters. +\item \code{\link[=require]{require()}} As an alternative, use roxygen2's @importFrom statement in packages and \code{\link[=library]{library()}} or \code{::} in scripts, instead of modifying the global search path. +\item \code{\link[=sapply]{sapply()}} As an alternative, use \code{\link[=vapply]{vapply()}} with an appropriate \code{FUN.VALUE=} argument to obtain type-stable simplification. +\item \code{\link[=setwd]{setwd()}} As an alternative, use \code{\link[withr:with_dir]{withr::with_dir()}} for a temporary change instead of modifying the global working directory. +\item \code{\link[=sink]{sink()}} As an alternative, use \code{\link[withr:with_sink]{withr::with_sink()}} for a temporary redirection instead of permanently redirecting output. +\item \code{\link[=source]{source()}} As an alternative, manage dependencies through packages. \code{\link[=source]{source()}} loads code into the global environment unless \code{local = TRUE} is used, which can cause hard-to-predict behavior. +\item \code{\link[=structure]{structure()}} As an alternative, Use \code{class<-}, \code{names<-}, and \code{attr<-} to set attributes. +\item \code{\link[=Sys.setenv]{Sys.setenv()}} As an alternative, use \code{\link[withr:with_envvar]{withr::with_envvar()}} for a temporary change instead of permanently modifying global environment variables. +\item \code{\link[=Sys.setlocale]{Sys.setlocale()}} As an alternative, use \code{\link[withr:with_locale]{withr::with_locale()}} for a temporary change instead of permanently modifying the session locale. +\item \code{\link[=trace]{trace()}} As an alternative, remove this likely leftover from debugging. It traps a function and causes execution of arbitrary code when that function is run. +\item \code{\link[=undebug]{undebug()}} As an alternative, remove this likely leftover from debugging. It is only useful for interactive debugging with \code{\link[=debug]{debug()}}. +\item \code{\link[=untrace]{untrace()}} As an alternative, remove this likely leftover from debugging. It is only useful for interactive debugging with \code{\link[=trace]{trace()}}. } The following operators are sometimes regarded as undesirable: \itemize{ -\item \code{\link[base:ns-dblcolon]{:::}} accesses non-exported functions inside packages. Code relying on these is -likely to break in future versions of the package because the functions are not part of the public interface and -may be changed or removed by the maintainers without notice. -Use public functions via \code{::} instead. -\item \code{\link[base:assignOps]{<<-}} and \verb{->>} assign outside the current environment in a way that can be hard to reason -about. Prefer fully-encapsulated functions wherever possible, or, if necessary, assign to a specific environment -with \code{\link[=assign]{assign()}}. Recall that you can create an environment at the desired scope with \code{\link[=new.env]{new.env()}}. +\item \code{\link[base:assignOps]{<<-}} As an alternative, It assigns outside the current environment in a way that can be hard to reason about. Prefer fully-encapsulated functions wherever possible, or, if necessary, assign to a specific environment with \code{\link[=assign]{assign()}}. Recall that you can create an environment at the desired scope with \code{\link[=new.env]{new.env()}}. +\item \code{\link[base:assignOps]{<<-}} As an alternative, It assigns outside the current environment in a way that can be hard to reason about. Prefer fully-encapsulated functions wherever possible, or, if necessary, assign to a specific environment with \code{\link[=assign]{assign()}}. Recall that you can create an environment at the desired scope with \code{\link[=new.env]{new.env()}}. +\item \code{\link[base:ns-dblcolon]{:::}} As an alternative, It accesses non-exported functions inside packages. Code relying on these is likely to break in future versions of the package because the functions are not part of the public interface and may be changed or removed by the maintainers without notice. Use public functions via \code{::} instead. } } -\keyword{datasets}