Run linters on test scripts (#6121)

The default pattern for files to lint is `pattern = "(?i)[.](r|rmd|qmd|rnw|rhtml|rrst|rtex|rtxt)$"`, i.e. not `Rraw`. We don't need the other extensions, so trim them also for simplicity.

See:

https://lintr.r-lib.org/reference/lint.html

Author: MichaelChirico

.ci/.lintr.R

@@ -31,6 +31,7 @@ linters = c(dt_linters, all_linters(
   )),
   # TODO(lintr#2441): Use upstream implementation.
   assignment_linter = NULL,
+  absolute_path_linter = NULL, # too many false positives
   # TODO(lintr#2442): Use this once x[ , j, by] is supported.
   commas_linter = NULL,
   commented_code_linter = NULL,
@@ -89,9 +90,9 @@ linters = c(dt_linters, all_linters(
 rm(dt_linters)
 
 # TODO(lintr#2172): Glob with lintr itself.
-exclusions = local({
+exclusions = c(local({
   exclusion_for_dir <- function(dir, exclusions) {
-    files = list.files(dir, pattern = "\\.(R|Rmd)$")
+    files = file.path("..", list.files(dir, pattern = "\\.(R|Rmd|Rraw)$", full.names=TRUE))
     stats::setNames(rep(list(exclusions), length(files)), files)
   }
   c(
@@ -106,6 +107,18 @@ exclusions = local({
       quotes_linter = Inf
       # strings_as_factors_linter = Inf
       # system_time_linter = Inf
+    )),
+    exclusion_for_dir("inst/tests", list(
+      library_call_linter = Inf,
+      numeric_leading_zero_linter = Inf,
+      undesirable_operator_linter = Inf, # For ':::', possibly we could be more careful to only exclude ':::'.
+      # TODO(michaelchirico): Enforce these and re-activate them one-by-one.
+      comparison_negation_linter = Inf,
+      duplicate_argument_linter = Inf,
+      equals_na_linter = Inf,
+      paste_linter = Inf
     ))
   )
-})
+}),
+  list(`../inst/tests/froll.Rraw` = list(dt_test_literal_linter = Inf)) # TODO(michaelchirico): Fix these once #5898, #5692, #5682, #5576, #5575, #5441 are merged.
+)

.github/workflows/lint.yaml

@@ -28,7 +28,7 @@ jobs:
           needs: lint
 
       - name: Lint
-        run: lintr::lint_package()
+        run: lintr::lint_package(pattern = "(?i)[.](r|rmd|rraw)$") # TODO(#5830): use the default pattern
         shell: Rscript {0}
         env:
           LINTR_ERROR_ON_LINT: true

inst/tests/benchmark.Rraw

@@ -60,7 +60,7 @@ test(647, ans1, ans3)
 # this'll error with `valgrind` because of the 'long double' usage in gsumm.c (although I wonder if we need long double precision).
 # http://valgrind.org/docs/manual/manual-core.html#manual-core.limits
 # http://comments.gmane.org/gmane.comp.debugging.valgrind/10340
-test(648, any(is.na(ans1$V1)) && !any(is.nan(ans1$V1)))
+test(648, anyNA(ans1$V1) && !any(is.nan(ans1$V1)))
 # test 649 removed as compared 1.1s to 1.1s
 if (.devtesting) test(650, tt1["user.self"] < tt3["user.self"])
 
@@ -500,5 +500,3 @@ start = gc()["Vcells",2]
 for (i in 1:10) data.table::fread("out.tsv")
 end = gc()["Vcells",2]
 test(, end/start < 1.05)
-
-

inst/tests/nafill.Rraw

@@ -345,4 +345,3 @@ test(99.3, data.table(a=1,b=2)[1,1, verbose=NA], error="verbose must be length 1
 options(datatable.verbose=1)
 test(99.4, coerceAs(1, 2L), error="verbose option must be length 1 non-NA logical or integer")
 options(datatable.verbose=FALSE)
-

inst/tests/other.Rraw

@@ -20,13 +20,13 @@ if (exists("test.data.table",.GlobalEnv,inherits=FALSE) ||
 test = data.table:::test
 INT = data.table:::INT
 
-if (any(duplicated(pkgs))) stop("Packages defined to be loaded for integration tests in 'inst/tests/other.Rraw' contains duplicates.")
+if (anyDuplicated(pkgs)) stop("Packages defined to be loaded for integration tests in 'inst/tests/other.Rraw' contains duplicates.")
 
 f = function(pkg) suppressWarnings(suppressMessages(isTRUE(
   library(pkg, character.only=TRUE, logical.return=TRUE, quietly=TRUE, warn.conflicts=FALSE, pos="package:base")  # attach at the end for #5101
 )))
 loaded = sapply(pkgs, f)
-if (any(!loaded)) {
+if (!all(loaded)) {
   stop("test.data.table('other.Rraw') is missing required package(s): ", toString(names(loaded)[!loaded]), ". If you can't install them and this is R CMD check, please set environment variable TEST_DATA_TABLE_WITH_OTHER_PACKAGES back to the default, false.")
   # Would like to install them now for convenience but gitlab-ci.yml seems to install to bus/mirror-other-packages/cran.
   # If that's a cache, that's nice, but we don't know at this point whether this script is being run by GLCI or by a user or in dev.
@@ -646,7 +646,7 @@ if (loaded[["nanotime"]]) {
   test(21.2, shift(x, fill=0L),           c(nanotime::nanotime(0L), x[1:3]))
   test(21.3, shift(x, 1,  type="cyclic"), c(x[4L], x[-4L]))
   test(21.4, shift(x, -1, type="cyclic"), c(x[-1L], x[1L]))
-  
+
   # was 1752 in tests.Rraw, #5516
   DT = data.table(A=nanotime(tt<-c("2016-09-28T15:30:00.000000070Z",
                                    "2016-09-29T23:59:00.000000001Z",
@@ -661,7 +661,7 @@ if (loaded[["nanotime"]]) {
                                    "1967-03-15T00:00:00.300000002Z",
                                    "1967-03-15T23:59:59.300000002Z")))
   test(22, capture.output(fwrite(DT, verbose=FALSE))[-1], tt)
-  
+
   # was 2060.401-405 in tests.Rraw, #5516
   nt = nanotime(c(1L, 2L, NA_integer_, 4L))
   nt_val = nanotime(1:4)
@@ -670,7 +670,7 @@ if (loaded[["nanotime"]]) {
   test(23.3, as.character(fcoalesce(nt, nanotime(rep(3, 4L)))), as.character(nt_val))
   test(23.4, fcoalesce(nt, 1), error='Item 2 has a different class than item 1')
   test(23.5, fcoalesce(nt, 1L), error = 'Item 2 is type integer but the first item is type double')
-  
+
   # was 2080.01-05 in tests.Rraw, #5516
   n = nanotime(1:4)
   n[2L] = NA
@@ -681,15 +681,15 @@ if (loaded[["nanotime"]]) {
   options(opt)
   test(24.4, between(1:10, nanotime(3), nanotime(6)), error="x is not integer64 but.*Please align classes")
   test(24.5, between(1:10, 3, nanotime(6)),           error="x is not integer64 but.*Please align classes")
-  
+
   # was 2085.11 in tests.Rraw, #5516
   n = nanotime(1:4)
   test(25, fifelse(c(TRUE,FALSE,NA,TRUE), n, n+100), c(n[1L], n[2L]+100, nanotime(NA), n[4]))
 
   # was 2127.27 in tests.Rraw, #5516
   n = nanotime(1:12)
   test(26, fcase(c(-5L:5L<0L,NA), n, c(-5L:5L>0L,NA), n+100), c(n[1L:5L], nanotime(NA), n[7L:11L]+100, as.integer64(NA)))
-  
+
   # na.omit works for nanotime, #4744. Was 2205 in tests.Rraw, #5516
   DT = data.table(time=nanotime(c(1,NA,3)))
   test(27, na.omit(DT), DT[c(1,3)])
@@ -728,5 +728,5 @@ if (FALSE) {  # moved from tests.Rraw in #5517 and not yet back on; wasn't sure
 if (loaded[["dplyr"]]) {
   # regression test for converting character->list column in a magrittr (dplyr) pipe, #2651
   DT = data.table(a = 1, b = 2, c = '1,2,3,4', d = 4)
-  test(30, DT[, c := strsplit(c, ',', fixed = TRUE) %>% lapply(as.integer) %>% as.list]$c, list(1:4))
+  test(30, DT[, c := strsplit(c, ',', fixed = TRUE) %>% lapply(as.integer) %>% as.list]$c, list(1:4)) # nolint: pipe_call_linter. Mimicking MRE as filed.
 }