Merge pull request #698 from rstudio/aron-lintr

initial lintr configuration

.Rbuildignore

@@ -9,6 +9,7 @@
 ^RELEASE\.md$
 ^\.Rprofile$
 ^\.github$
+^\.lintr$
 ^tags$
 ^R/tags$
 ^tests/testthat/packrat/lib$

.github/workflows/lint.yaml

@@ -0,0 +1,32 @@
+# Workflow derived from https://github.com/r-lib/actions/tree/v2/examples
+# Need help debugging build failures? Start at https://github.com/r-lib/actions#where-to-find-help
+on:
+  push:
+    branches: [main, master]
+  pull_request:
+    branches: [main, master]
+
+name: lint
+
+jobs:
+  lint:
+    runs-on: ubuntu-latest
+    env:
+      GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
+    steps:
+      - uses: actions/checkout@v3
+
+      - uses: r-lib/actions/setup-r@v2
+        with:
+          use-public-rspm: true
+
+      - uses: r-lib/actions/setup-r-dependencies@v2
+        with:
+          extra-packages: any::lintr, local::.
+          needs: lint
+
+      - name: Lint
+        run: lintr::lint_package()
+        shell: Rscript {0}
+        env:
+          LINTR_ERROR_ON_LINT: true

.lintr

@@ -0,0 +1,24 @@
+linters: linters_with_defaults(
+    # some tests have _VERY_ long expectations.
+    line_length_linter(270),
+    object_length_linter(80),
+    object_name_linter = NULL,
+    object_usage_linter = NULL,
+    brace_linter = NULL,
+    commented_code_linter = NULL,
+    cyclocomp_linter = NULL,
+    single_quotes_linter = NULL,
+    seq_linter = NULL
+  )
+exclusions: list(
+    # renv is vendored code, not ours.
+    "inst/resources/renv.R",
+    # test resources that are not "source".
+    "tests/testthat/lockfiles",
+    "tests/testthat/other-packages",
+    "tests/testthat/packrat",
+    "tests/testthat/projects",
+    "tests/testthat/repo",
+    "tests/testthat/repo-empty",
+    "tests/testthat/resources"
+  )

R/aaa-globals.R

@@ -5,32 +5,32 @@
 
 ## Mutable values that might be modified by the user (code borrowed from knitr)
 # merge elements of y into x with the same names
-merge_list = function(x, y) {
-  x[names(y)] = y
+merge_list <- function(x, y) {
+  x[names(y)] <- y
   x
 }
 
-new_defaults = function(value = list()) {
-  defaults = value
+new_defaults <- function(value = list()) {
+  defaults <- value
 
-  get = function(name, default = FALSE, drop = TRUE) {
-    if (default) defaults = value  # this is only a local version
+  get <- function(name, default = FALSE, drop = TRUE) {
+    if (default) defaults <- value  # this is only a local version
     if (missing(name)) defaults else {
       if (drop && length(name) == 1) defaults[[name]] else {
         setNames(defaults[name], name)
       }
     }
   }
-  set = function(...) {
-    dots = list(...)
+  set <- function(...) {
+    dots <- list(...)
     if (length(dots) == 0) return()
     if (is.null(names(dots)) && length(dots) == 1 && is.list(dots[[1]]))
       if (length(dots <- dots[[1]]) == 0) return()
     defaults <<- merge(dots)
     invisible(NULL)
   }
-  merge = function(values) merge_list(defaults, values)
-  restore = function(target = value) defaults <<- target
+  merge <- function(values) merge_list(defaults, values)
+  restore <- function(target = value) defaults <<- target
 
   list(get = get, set = set, merge = merge, restore = restore)
 }

R/augment-rprofile.R

@@ -47,4 +47,3 @@ editRprofileAutoloader <- function(project, action = c("update", "remove")) {
 
   invisible()
 }
-

R/available-updates.R

@@ -87,7 +87,7 @@ bitbucketUpdates <- function(lib.loc = .libPaths()) {
     })
     names(DESCRIPTIONS) <- pkgs
     DESCRIPTIONS <-
-      Filter(function(x) "RemoteType" %in% colnames(x) && x[,"RemoteType"] == "bitbucket", DESCRIPTIONS)
+      Filter(function(x) "RemoteType" %in% colnames(x) && x[, "RemoteType"] == "bitbucket", DESCRIPTIONS)
     if (!length(DESCRIPTIONS)) return(NULL)
     if (!requireNamespace("httr")) stop("Need package 'httr' to check for Bitbucket updates")
     do.call(rbind, enumerate(DESCRIPTIONS, function(x) {
@@ -152,7 +152,7 @@ gitlabUpdates <- function(lib.loc = .libPaths()) {
     names(DESCRIPTIONS) <- pkgs
     DESCRIPTIONS <-
       Filter(function(x) "RemoteType" %in% colnames(x) &&
-               x[,"RemoteType"] == "gitlab", DESCRIPTIONS)
+               x[, "RemoteType"] == "gitlab", DESCRIPTIONS)
     if (!length(DESCRIPTIONS)) return(NULL)
     if (!requireNamespace("httr")) stop("Need package 'httr' to check for Gitlab updates")
     do.call(rbind, enumerate(DESCRIPTIONS, function(x) {

R/bitbucket.R

@@ -33,7 +33,7 @@ bitbucketDownloadHttr <- function(url, destfile, ...) {
 
   user <- bitbucket_user(quiet = TRUE)
   pwd <- bitbucket_pwd(quiet = TRUE)
-  auth <- if (!is.null(user) & !is.null(pwd)) {
+  auth <- if (!is.null(user) && !is.null(pwd)) {
     authenticate(user, pwd, type = "basic")
   } else {
     list()

R/bundle.R

@@ -6,7 +6,7 @@
 #' be unbundled either with \code{packrat::\link{unbundle}} (which
 #' restores the project as well), \R's own \code{utils::\link{untar}}, or
 #' through most system \code{tar} implementations.
-#' 
+#'
 #' The tar binary is selected using the same heuristic as \code{\link{restore}}.
 #'
 #' @param project The project directory. Defaults to the currently activate

R/dependencies.R

@@ -221,7 +221,7 @@ dirDependenciesBuiltIn <- function(dir) {
       "(?:^",
       paste0(
         ignoredDir,
-        collapse=")|(?:^"
+        collapse = ")|(?:^"
       ),
       ")"
     )
@@ -828,7 +828,7 @@ fileDependencies.tangle <- function(file, encoding = "UTF-8") {
   # parse each r chunk independently to retrieve dependencies
   # allows for some chunks to be _broken_ but not stop retrieving dependencies
   r_chunks <- strsplit(paste0(readLines(outfile), collapse = "\n"), key)[[1]]
-  for(r_chunk in r_chunks) {
+  for (r_chunk in r_chunks) {
     try(silent = TRUE, {
       parsed <- parse(text = r_chunk, encoding = encoding)
       deps <- c(deps, expressionDependencies(parsed))

R/external.R

@@ -100,7 +100,7 @@ extlib <- function(packages) {
 loadExternalPackages <- function() {
   pkgs <- get_opts("external.packages")
   if (length(pkgs)) {
-    pkgs <- pkgs[ !is.null(pkgs) & !is.na(pkgs) & nchar(pkgs) ]
+    pkgs <- pkgs[!is.null(pkgs) & !is.na(pkgs) & nchar(pkgs)]
     failures <- dropNull(lapply(pkgs, function(pkg) {
       tryCatch(
         expr = extlib(pkg),

R/get-package-actions.R

@@ -101,4 +101,3 @@ packageActionMessages <- function(verb, records) {
   }
   msgs
 }
-

R/gitlab.R

@@ -99,4 +99,3 @@ gitlab_pat <- function(quiet = TRUE) {
   }
   return(NULL)
 }
-

R/hooks.R

@@ -17,9 +17,9 @@ snapshotHook <- function(expr, value, ok, visible) {
       project <- .packrat_mutables$get("project")
 
       if (is.null(project)) {
-        file = "" ## to stdout
+        file <- "" ## to stdout
       } else {
-        file = file.path(project, "packrat", "packrat.log")
+        file <- file.path(project, "packrat", "packrat.log")
       }
 
       if (inherits(e, "simpleError")) {

R/install.R

@@ -500,7 +500,7 @@ scan_registry_for_rtools <- function(debug = FALSE) {
   for (i in seq_along(keys)) {
     version <- names(keys)[[i]]
     key <- keys[[version]]
-    if (!is.list(key) || is.null(key$InstallPath)) next;
+    if (!is.list(key) || is.null(key$InstallPath)) next
     install_path <- normalizePath(key$InstallPath,
                                   mustWork = FALSE, winslash = "/")
 

R/lockfile-metadata.R

@@ -102,5 +102,3 @@ available_metadata <- c(
   r_version = "RVersion",
   repos = "Repos"
 )
-
-

R/lockfile.R

@@ -235,7 +235,7 @@ deserializePackages <- function(df) {
   df <- df[, names(df) != 'requires', drop = FALSE]
 
   sortedPackages <- lapply(topoSorted, function(pkgName) {
-    pkg <- as.list(df[df$name == pkgName,])
+    pkg <- as.list(df[df$name == pkgName, ])
     pkg <- pkg[!is.na(pkg)]
     return(pkg)
   })

R/packrat.R

@@ -601,7 +601,7 @@ packify <- function(project = NULL, quiet = FALSE) {
   invisible()
 }
 
-lockInfo <- function(project, property='packages', fatal=TRUE) {
+lockInfo <- function(project, property = 'packages', fatal = TRUE) {
 
   project <- getProjectDir(project)
 

R/pkg.R

@@ -133,7 +133,7 @@ getPackageRecordsExternalSource <- function(pkgNames,
       # Construct the package record by hand -- generate the minimal
       # bits of the DESCRIPTION file, and infer the package record
       # from that.
-      pkg <- available[pkgName,]
+      pkg <- available[pkgName, ]
       df <- data.frame(
         Package = pkg[["Package"]],
         Version = pkg[["Version"]],
@@ -319,7 +319,7 @@ getPackageRecords <- function(pkgNames,
 # Return TRUE when the data frame for this package has the given RemoteType.
 hasRemoteType <- function(df, remoteType) {
   # Do not compare with 'identical'; RemoteType may be a factor.
-  return (!is.null(df$RemoteType) && df$RemoteType == remoteType)
+  return(!is.null(df$RemoteType) && df$RemoteType == remoteType)
 }
 
 # Reads a description file and attempts to infer where the package came from.
@@ -577,7 +577,7 @@ diffableRecord <- function(record) {
 # debug helper to print a package record. includes field names, type of value, and value.
 printPackageRecord <- function(name, record) {
   cat(name, "\n")
-  cat(paste(names(record), lapply(record,typeof), record, sep = ":", collapse = "\n"),"\n")
+  cat(paste(names(record), lapply(record, typeof), record, sep = ":", collapse = "\n"), "\n")
 }
 
 # states: NA (unchanged), remove, add, upgrade, downgrade, crossgrade

R/pretty-print.R

@@ -62,7 +62,7 @@ prettyPrintPair <- function(packagesFrom, packagesTo, header, footer = NULL,
     }
     for (i in seq_along(packagesFrom)) {
       if (!is.null(packagesFrom[[i]]) && !is.null(packagesTo[[i]])) {
-        if (!identical(packagesFrom[[i]]$name , packagesTo[[i]]$name)) {
+        if (!identical(packagesFrom[[i]]$name, packagesTo[[i]]$name)) {
           stop('Invalid arguments--package names did not match')
         }
       }

R/restore.R

@@ -35,7 +35,7 @@ isFromCranlikeRepo <- function(pkgRecord, repos) {
 # the package version is current. NB: Assumes package is present in db.
 versionMatchesDb <- function(pkgRecord, db) {
   versionMatch <-
-    identical(pkgRecord$version, db[pkgRecord$name,"Version"])
+    identical(pkgRecord$version, db[pkgRecord$name, "Version"])
 
   # For GitHub, Bitbucket, and Gitlab, we also need to check that the SHA1 is identical
   # (the source may be updated even if the version hasn't been bumped)
@@ -189,8 +189,8 @@ getSourceForPkgRecord <- function(pkgRecord,
       # If the file wasn't saved to the destination directory (which can happen
       # if the repo is local--see documentation in download.packages), copy it
       # there now
-      if (!identical(fileLoc[1,2], file.path(pkgSrcDir, pkgSrcFile))) {
-        file.copy(fileLoc[1,2], pkgSrcDir)
+      if (!identical(fileLoc[1, 2], file.path(pkgSrcDir, pkgSrcFile))) {
+        file.copy(fileLoc[1, 2], pkgSrcDir)
       }
       type <- paste(type, "current")
     } else {
@@ -439,7 +439,7 @@ annotatePkgs <- function(pkgNames, project, lib = libDir(project)) {
 
 # Takes a vector of package names, and returns a logical vector that indicates
 # whether the package was not installed by packrat.
-installedByPackrat <- function(pkgNames, lib.loc, default=NA) {
+installedByPackrat <- function(pkgNames, lib.loc, default = NA) {
   # Can't use installed.packages(fields='InstallAgent') here because it uses
   # Meta/package.rds, not the DESCRIPTION file, and we only record this info in
   # the DESCRIPTION file.
@@ -678,7 +678,7 @@ playActions <- function(pkgRecords, actions, repos, project, lib) {
       # Changing package type or version: Remove the old one now (we'll write
       # a new one in a moment)
       message("Replacing ", pkgRecord$name, " (", action, " ",
-              installedPkgs[pkgRecord$name,"Version"], " to ",
+              installedPkgs[pkgRecord$name, "Version"], " to ",
               pkgRecord$version, ") ... ", appendLF = FALSE)
       removePkgs(project, pkgRecord$name, lib)
     } else if (identical(action, "add")) {
@@ -1009,4 +1009,3 @@ appendRemoteInfoToDescription <- function(src, dest, remote_info) {
 
   return(TRUE)
 }
-

R/rstudio-protocol.R

@@ -12,4 +12,4 @@
 #   the protocol version number, is used to determine whether the package is too
 #   old to be compatible.
 
-.RStudio_protocol_version = 1
+.RStudio_protocol_version <- 1

R/search-path.R

@@ -16,7 +16,7 @@ search_path <- function() {
   )
 
   ## Filter to only actual packages
-  pkgs <- pkgs[ grep("^package:", pkgs$package), , drop = FALSE]
+  pkgs <- pkgs[grep("^package:", pkgs$package), , drop = FALSE]
 
   ## Clean up the package name by removing the initial 'package:'
   pkgs$package <- sub("^package:", "", pkgs$package)
@@ -37,7 +37,7 @@ search_path <- function() {
   pkgs$lib.dir <- dirname(pkgs$lib.loc)
 
   ## Arrange by lib.dir
-  pkgs <- pkgs[ order(pkgs$lib.dir), ]
+  pkgs <- pkgs[order(pkgs$lib.dir), ]
 
   ## Unset the rownames
   rownames(pkgs) <- NULL

R/utils.R

@@ -95,7 +95,7 @@ dir_copy <- function(from, to, overwrite = FALSE, all.files = TRUE,
     if (overwrite) {
       unlink(to, recursive = TRUE)
     } else {
-      stop(paste( sep = "",
+      stop(paste(sep = "",
                   if (is_dir(to)) "Directory" else "File",
                   " already exists at path '", to, "'."
       ))

tests/testthat/test-hash.R

@@ -14,9 +14,14 @@ test_that("hash function is available and has expected arguments", {
   # which expects this non-exported function to be present. That's not to say
   # the function signature can't be changed, only that the relevant call in
   # Connect's packrat_restore.R should be studied to avoid any breakage.
+
+
   expect_identical(
     formals(hash),
-    pairlist(path = quote(expr =), descLookup = as.name("installedDescLookup"))
+    pairlist(
+      path = quote(expr =), # nolint: infix_spaces_linter. r-lib/lintr#1889
+      descLookup = as.name("installedDescLookup")
+      )
   )
 
 })

tests/testthat/test-packrat.R

@@ -28,7 +28,7 @@ withTestContext({
     skip_on_cran()
     projRoot <- cloneTestProject("sated")
     init(enter = FALSE, projRoot, options = list(local.repos = "packages"),
-         infer.dependencies=FALSE)
+         infer.dependencies = FALSE)
     lib <- libDir(projRoot)
     expect_true(file.exists(lockFilePath(projRoot)))
     expect_true(file.exists(srcDir(projRoot)))
@@ -137,7 +137,7 @@ withTestContext({
     skip_on_cran()
     projRoot <- cloneTestProject("partlyignored")
     lib <- libDir(projRoot)
-    init(enter = FALSE, projRoot, options = list(ignored.directories="ignoreme"))
+    init(enter = FALSE, projRoot, options = list(ignored.directories = "ignoreme"))
 
     # This test project has a file called notignored.R that depends on bread, and
     # another file called ignoreme/ignorethis.R that depends on toast.

tests/testthat/test-shiny.R

@@ -1,7 +1,7 @@
 test_that("Shiny examples have a shiny dependency", {
   skip_on_cran()
   skip_if_not_installed("shiny")
-  
+
   # Confirm packrat believes all example shiny apps are, in fact, shiny apps
   examplesPath <- system.file("examples", package = "shiny")
   apps <- list.files(examplesPath, full.names = TRUE)
@@ -17,5 +17,4 @@ test_that("projects which use shiny implicitly are detected", {
   # R Markdown document with 'runtime: shiny'
   interactiveDocPath <- file.path("resources", "interactive-doc-example.Rmd")
   expect_true("shiny" %in% packrat:::fileDependencies(interactiveDocPath))
-
 })