Rdatatable · MichaelChirico · Oct 26, 2020 · Dec 14, 2023 · Dec 14, 2023 · Dec 14, 2023
@@ -0,0 +1,32 @@
+on:
+  push:
+    branches: master
+  pull_request:
+    branches: master
+
+name: lint
+
+jobs:
+  lint:
+    runs-on: ubuntu-latest
+    env:
+      GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: r-lib/actions/setup-r@v2
+        with:
+          use-public-rspm: true
+
+      - uses: r-lib/actions/setup-r-dependencies@v2
+        with:
+          extra-packages: |
+            r-lib/lintr
+            local::.
+          needs: lint
+
+      - name: Lint
+        run: lintr::lint_package()
+        shell: Rscript {0}
+        env:
+          LINTR_ERROR_ON_LINT: true
@@ -0,0 +1,92 @@
+for (f in list.files('ci/linters', full.names=TRUE)) source(f)
+rm(f)
+
+linters = all_linters(
+  packages = "lintr", # TODO(lintr->3.2.0): Remove this.
+  # eq_assignment_linter(),
+  brace_linter(allow_single_line = TRUE),
+  # implicit_assignment_linter(allow_lazy = TRUE, allow_scoped = TRUE),
+  # implicit_integer_linter(allow_colon = TRUE),
+  # system_time_linter = undesirable_function_linter(c(
+  #   system.time = "Only run timings in benchmark.Rraw"
+  # )),
+  # undesirable_function_linter(modify_defaults(
+  #   default_undesirable_functions,
+  #   ifelse = "Use fifelse instead.",
+  #   Sys.setenv = NULL,
+  #   library = NULL,
+  #   options = NULL,
+  #   par = NULL,
+  #   setwd = NULL
+  # )),
+  undesirable_operator_linter(modify_defaults(
+    default_undesirable_operators,
+    `<<-` = NULL
+  )),
+  # TODO(lintr#2441): Use upstream implementation.
+  assignment_linter = NULL,
+  # TODO(lintr#2442): Use this once x[ , j, by] is supported.
+  commas_linter = NULL,
+  commented_code_linter = NULL,
+  # TODO(linter->3.2.0): Activate this.
+  consecutive_assertion_linter = NULL,
+  cyclocomp_linter = NULL,
+  # TODO(linter->3.2.0): Remove this.
+  extraction_operator_linter = NULL,
+  function_argument_linter = NULL,
+  indentation_linter = NULL,
+  infix_spaces_linter = NULL,
+  # TODO(R>3.2.0): Activate this, extending to recognize vapply_1i(x, length).
+  lengths_linter = NULL,
+  line_length_linter = NULL,
+  missing_package_linter = NULL,
+  namespace_linter = NULL,
+  nonportable_path_linter = NULL,
+  object_name_linter = NULL,
+  object_usage_linter = NULL,
+  quotes_linter = NULL,
+  semicolon_linter = NULL,
+  spaces_inside_linter = NULL,
+  spaces_left_parentheses_linter = NULL,
+  # TODO(michaelchirico): Only exclude from vignettes, not sure what's wrong.
+  strings_as_factors_linter = NULL,
+  # TODO(lintr->3.2.0): Fix on a valid TODO style, enforce it, and re-activate.
+  todo_comment_linter = NULL,
+  # TODO(lintr#2443): Use this.
+  unnecessary_nested_if_linter = NULL,
+  # TODO(michaelchirico): Enforce these and re-activate them one-by-one. Also stop using '<<-'.
+  brace_linter = NULL,
+  fixed_regex_linter = NULL,
+  function_left_parentheses_linter = NULL,
+  if_not_else_linter = NULL,
+  implicit_assignment_linter = NULL,
+  implicit_integer_linter = NULL,
+  keyword_quote_linter = NULL,
+  paren_body_linter = NULL,
+  redundant_equals_linter = NULL,
+  scalar_in_linter = NULL,
+  undesirable_function_linter = NULL,
+  unnecessary_concatenation_linter = NULL,
+  unreachable_code_linter = NULL
+)
+# TODO(lintr#2172): Glob with lintr itself.
+exclusions = local({
+  exclusion_for_dir <- function(dir, exclusions) {
+    files = list.files(dir, pattern = "\\.(R|Rmd)$")
+    stats::setNames(rep(list(exclusions), length(files)), files)
+  }
+  c(
+    exclusion_for_dir("tests", list(
+      quotes_linter = Inf,
+      # TODO(michaelchirico): Enforce these and re-activate them one-by-one.
+      implicit_integer_linter = Inf,
+      infix_spaces_linter = Inf,
+      undesirable_function_linter = Inf
+    )),
+    exclusion_for_dir("vignettes", list(
+      quotes_linter = Inf
+      # strings_as_factors_linter = Inf
+      # system_time_linter = Inf
+    ))
+  )
+})
@@ -187,4 +187,3 @@ bmerge = function(i, x, icols, xcols, roll, rollends, nomatch, mult, ops, verbos
   ans$xo = xo  # for further use by [.data.table
   return(ans)
 }
-
@@ -1920,7 +1920,7 @@ replace_dot_alias = function(e) {
     attrs = attr(x, 'index', exact=TRUE)
     skeys = names(attributes(attrs))
     if (!is.null(skeys)) {
-      hits  = unlist(lapply(paste0("__", names_x[cols]), function(x) grep(x, skeys, fixed = TRUE)))
+      hits  = unlist(lapply(paste0("__", names_x[cols]), grep, skeys, fixed = TRUE))
       hits  = skeys[unique(hits)]
       for (i in seq_along(hits)) setattr(attrs, hits[i], NULL) # does by reference
     }
@@ -2084,7 +2084,7 @@ as.matrix.data.table = function(x, rownames=NULL, rownames.value=NULL, ...) {
     }
     if (!is.logical(xj))
       all.logical = FALSE
-    if (length(levels(xj)) > 0L || !(is.numeric(xj) || is.complex(xj) || is.logical(xj)) ||
+    if (nlevels(xj) > 0L || !(is.numeric(xj) || is.complex(xj) || is.logical(xj)) ||
         (!is.null(cl <- attr(xj, "class", exact=TRUE)) && any(cl %chin%
         c("Date", "POSIXct", "POSIXlt"))))
       non.numeric = TRUE
@@ -2104,7 +2104,7 @@ as.matrix.data.table = function(x, rownames=NULL, rownames.value=NULL, ...) {
       if (is.character(X[[j]])) next
       xj = X[[j]]
       miss = is.na(xj)
-      xj = if (length(levels(xj))) as.vector(xj) else format(xj)
+      xj = if (nlevels(xj)) as.vector(xj) else format(xj)
       is.na(xj) = miss
       X[[j]] = xj
     }

@@ -118,4 +118,3 @@ uniqueN = function(x, by = if (is.list(x)) seq_along(x) else NULL, na.rm=FALSE)
     length(starts)
   }
 }
-
@@ -1,7 +1,7 @@
 # 647 fast droplevels.data.table method
 fdroplevels = function(x, exclude = if (anyNA(levels(x))) NULL else NA, ...) {
   stopifnot(inherits(x, "factor"))
-  lev = which(tabulate(x, length(levels(x))) & (!match(levels(x), exclude, 0L)))
+  lev = which(tabulate(x, nlevels(x)) & (!match(levels(x), exclude, 0L)))
   ans = match(as.integer(x), lev)
   setattr(ans, 'levels', levels(x)[lev])
   setattr(ans, 'class', class(x))

@@ -28,7 +28,7 @@ patterns = function(..., cols=character(0L)) {
     stopf("Input patterns must be of type character.")
   matched = lapply(p, grep, cols)
   # replace with lengths when R 3.2.0 dependency arrives
-  if (length(idx <- which(sapply(matched, length) == 0L)))
+  if (length(idx <- which(vapply_1i(matched, length) == 0L)))
     stopf('Pattern(s) not found: [%s]', brackify(p[idx]))
   matched
 }
@@ -123,7 +123,7 @@ measurev = function(fun.list, sep="_", pattern, cols, multiple.keyword="value.na
       stopf("sep must be character string")
     }
     list.of.vectors = strsplit(cols, sep, fixed=TRUE)
-    vector.lengths = sapply(list.of.vectors, length)
+    vector.lengths = vapply_1i(list.of.vectors, length)
     n.groups = max(vector.lengths)
     if (n.groups == 1) {
       stopf("each column name results in only one item after splitting using sep, which means that all columns would be melted; to fix please either specify melt on all columns directly without using measure, or use a different sep/pattern specification")

@@ -247,4 +247,3 @@ foverlaps = function(x, y, by.x=if (!is.null(key(x))) key(x) else key(y), by.y=k
 # Tests are added to ensure we cover these aspects (to my knowledge) to ensure that any undesirable changes in the future breaks those tests.
 
 # Conclusion: floating point manipulations are hell!
-
@@ -31,8 +31,8 @@ yaml=FALSE, autostart=NA, tmpdir=tempdir(), tz="UTC")
   if (is.na(nrows) || nrows<0) nrows=Inf   # accept -1 to mean Inf, as read.table does
   if (identical(header,"auto")) header=NA
   stopifnot(
-    is.logical(header) && length(header)==1L,  # TRUE, FALSE or NA
-    is.numeric(nThread) && length(nThread)==1L
+    "header should be a logical scalar" = is.logical(header) && length(header)==1L,  # TRUE, FALSE or NA
+    "nThread should be a logical scalar" = is.numeric(nThread) && length(nThread)==1L
   )
   nThread=as.integer(nThread)
   stopifnot(nThread>=1L)
@@ -185,7 +185,7 @@ yaml=FALSE, autostart=NA, tmpdir=tempdir(), tz="UTC")
 
     yaml_comment_re = '^#'
     yaml_string = character(0L)
-    while (TRUE) {
+    repeat {
       this_line = readLines(f, n=1L)
       n_read = n_read + 1L
       if (!length(this_line)){

@@ -58,7 +58,7 @@ fwrite = function(x, file="", append=FALSE, quote="auto",
     length(nThread)==1L && !is.na(nThread) && nThread>=1L
     )
 
-  is_gzip = compress == "gzip" || (compress == "auto" && grepl("\\.gz$", file))
+  is_gzip = compress == "gzip" || (compress == "auto" && endsWith(file, ".gz"))
 
   file = path.expand(file)  # "~/foo/bar"
   if (append && (file=="" || file.exists(file))) {
@@ -116,4 +116,3 @@ fwrite = function(x, file="", append=FALSE, quote="auto",
 }
 
 haszlib = function() .Call(Cdt_has_zlib)
-
@@ -21,7 +21,7 @@
     nth = getDTthreads(verbose=FALSE)
     if (dev)
       packageStartupMessagef("data.table %s IN DEVELOPMENT built %s%s using %d threads (see ?getDTthreads).  ", v, d, g, nth, appendLF=FALSE)
-    else 
+    else
       packageStartupMessagef("data.table %s using %d threads (see ?getDTthreads).  ", v, nth, appendLF=FALSE)
     packageStartupMessagef("Latest news: r-datatable.com")
     if (gettext("TRANSLATION CHECK") != "TRANSLATION CHECK")

@@ -13,4 +13,3 @@ setDTthreads = function(threads=NULL, restore_after_fork=NULL, percent=NULL, thr
 getDTthreads = function(verbose=getOption("datatable.verbose")) {
   .Call(CgetDTthreads, verbose)
 }
-
@@ -271,4 +271,3 @@ trunc_cols_message = function(not_printed, abbs, class, col.names){
     n, brackify(paste0(not_printed, classes))
   )
 }
-
@@ -18,7 +18,7 @@ list2lang = function(x) {
   char = vapply(x, is.character, FALSE)
   to.name = !asis & char
   if (any(to.name)) { ## turns "my_name" character scalar into `my_name` symbol, for convenience
-    if (any(non.scalar.char <- vapply(x[to.name], length, 0L)!=1L)) {
+    if (any(non.scalar.char <- vapply_1i(x[to.name], length)!=1L)) {
       stopf("Character objects provided in the input are not scalar objects, if you need them as character vector rather than a name, then wrap each into 'I' call: %s", brackify(names(non.scalar.char)[non.scalar.char]))
     }
     x[to.name] = lapply(x[to.name], as.name)

@@ -364,4 +364,3 @@ CJ = function(..., sorted = TRUE, unique = FALSE)
   }
   l
 }
-
@@ -290,4 +290,3 @@ all.equal.data.table = function(target, current, trim.levels=TRUE, check.attribu
   }
   TRUE
 }
-
@@ -12,7 +12,7 @@ type_size = function(DT) {
     tt = lookup[storage.mode(col)]
     if (is.na(tt)) tt = .Machine$sizeof.pointer
     tt = tt*nrow(DT)
-    if (is.factor(col)) tt = tt + length(levels(col))*.Machine$sizeof.pointer
+    if (is.factor(col)) tt = tt + nlevels(col)*.Machine$sizeof.pointer
     ans = ans + tt
   }
   ans + ncol(DT)*.Machine$sizeof.pointer  # column name pointers
@@ -60,4 +60,3 @@ tables = function(mb=type_size, order.col="NAME", width=80,
   }
   invisible(info)
 }
-
@@ -7,7 +7,7 @@ test.data.table = function(script="tests.Rraw", verbose=FALSE, pkg=".", silent=F
   if (length(memtest.id)) {
     if (length(memtest.id)==1L) memtest.id = rep(memtest.id, 2L)  # for convenience of supplying one id rather than always a range
     stopifnot(length(memtest.id)<=2L,  # conditions quoted to user when false so "<=2L" even though following conditions rely on ==2L
-                     !anyNA(memtest.id), memtest.id[1L]<=memtest.id[2L]) 
+                     !anyNA(memtest.id), memtest.id[1L]<=memtest.id[2L])
     if (memtest==0L) memtest=1L  # using memtest.id implies memtest
   }
   if (exists("test.data.table", .GlobalEnv, inherits=FALSE)) {
@@ -132,7 +132,7 @@ test.data.table = function(script="tests.Rraw", verbose=FALSE, pkg=".", silent=F
 
   owd = setwd(tempdir()) # ensure writeable directory; e.g. tests that plot may write .pdf here depending on device option and/or batch mode; #5190
   on.exit(setwd(owd))
-  
+
   if (memtest) {
     catf("\n***\n*** memtest=%d. This should be the first call in a fresh R_GC_MEM_GROW=0 R session for best results. Ctrl-C now if not.\n***\n\n", memtest)
     if (is.na(rss())) stopf("memtest intended for Linux. Step through data.table:::rss() to see what went wrong.")
@@ -243,7 +243,7 @@ INT = function(...) { as.integer(c(...)) }   # utility used in tests.Rraw
 gc_mem = function() {
   # nocov start
   # gc reports memory in MB
-  m = apply(gc()[, c(2L, 4L, 6L)], 2L, sum)
+  m = colSums(gc()[, c(2L, 4L, 6L)])
   names(m) = c("GC_used", "GC_gc_trigger", "GC_max_used")
   m
   # nocov end
@@ -461,4 +461,3 @@ test = function(num,x,y=TRUE,error=NULL,warning=NULL,message=NULL,output=NULL,no
   }
   invisible(!fail)
 }
-
@@ -12,4 +12,3 @@ timetaken = function(started.at)
   tt = proc.time()-started.at  # diff all 3 times
   paste0(format(tt[3L])," elapsed (", format(tt[1L]), " cpu)")
 }
-
@@ -56,7 +56,7 @@ tstrsplit = function(x, ..., fill=NA, type.convert=FALSE, keep, names=FALSE) {
         if (!(sum(!is_named) == 1L && !is_named[n] && is.function(type.convert[[n]])))
           stopf("When the argument 'type.convert' contains an unnamed element, it is expected to be the last element and should be a function. More than one unnamed element is not allowed unless all elements are functions with length equal to %d (the length of the transpose list or 'keep' argument if it is specified).", length(keep))
         else {
-          fothers = type.convert[[n]]         
+          fothers = type.convert[[n]]
           type.convert = type.convert[-n]
         }
       }
@@ -90,4 +90,3 @@ tstrsplit = function(x, ..., fill=NA, type.convert=FALSE, keep, names=FALSE) {
   setattr(ans, 'names', names)
   ans
 }
-
@@ -21,4 +21,3 @@ uniqlengths = function(x, len) {
   ans = .Call(Cuniqlengths, as.integer(x), as.integer(len))
   ans
 }
-
@@ -165,4 +165,3 @@ rss = function() {  #5515 #5517
   round(ans / 1024, 1L)  # return MB
   # nocov end
 }
-
@@ -14,24 +14,24 @@ print(DT[2,a:=4L])                    # no
 print(DT)                             # yes
 if (TRUE) DT[2,a:=5L]                 # no. used to print before v1.9.5
 if (TRUE) if (TRUE) DT[2,a:=6L]       # no. used to print before v1.9.5
-(function(){DT[2,a:=5L];NULL})()      # print NULL
+(function(){ DT[2,a:=5L]; NULL })()   # print NULL
 DT                                    # no (from v1.9.5+). := suppresses next auto print (can't distinguish just "DT" symbol alone at the prompt)
 DT                                    # yes. 2nd time needed, or solutions below
-(function(){DT[2,a:=5L];NULL})()      # print NULL
+(function(){ DT[2,a:=5L]; NULL })()   # print NULL
 DT[]                                  # yes. guaranteed print
-(function(){DT[2,a:=5L];NULL})()      # print NULL
+(function(){ DT[2,a:=5L]; NULL })()   # print NULL
 print(DT)                             # no. only DT[] is guaranteed print from v1.9.6 and R 3.2.0
-(function(){DT[2,a:=5L][];NULL})()    # print NULL
+(function(){ DT[2,a:=5L][]; NULL })() # print NULL
 DT                                    # yes. i) function needs to add [] after last one, so that "DT" alone is guaranteed anyway
-(function(){DT[2,a:=5L];DT[];NULL})() # print NULL
+(function(){ DT[2,a:=5L]; DT[]; NULL })() # print NULL
 DT                                    # yes. ii) or as a separate DT[] after the last := inside the function
 DT2 = data.table(b=3:4)               # no
-(function(){DT[2,a:=6L];DT2[1,b:=7L];NULL})()
+(function(){ DT[2,a:=6L]; DT2[1,b:=7L]; NULL })()
 DT                                    # yes. last := was on DT2 not DT
 {DT[2,a:=6L];invisible()}             # no
 print(DT)                             # no
-(function(){print(DT[2,a:=7L]);print(DT);invisible()})()    # yes*2
-{print(DT[2,a:=8L]);print(DT);invisible()}                  # yes*1  Not within function so as at prompt
+(function(){ print(DT[2,a:=7L]); print(DT); invisible() })()    # yes*2
+{ print(DT[2,a:=8L]); print(DT); invisible() }                  # yes*1  Not within function so as at prompt
 DT[1][,a:=9L]      # no (was too tricky to detect that DT[1] is a new object). Simple rule is that := always doesn't print
 DT[2,a:=10L][1]                       # yes
 DT[1,a:=10L][1,a:=10L]                # no
@@ -43,4 +43,3 @@ DT[1,a:=10L][]                        # yes. ...[] == oops, forgot print(...)
 tryCatch(DT[,foo:=ColumnNameTypo], error=function(e) e$message)         # error: not found.
 DT                                    # yes
 DT                                    # yes
-
@@ -6,4 +6,3 @@ if (suppressPackageStartupMessages(requireNamespace("knitr", quietly = TRUE))) {
 } else {
     cat(readLines("knitr.Rout.mock", warn = FALSE), sep="\n")
 }
-
@@ -9,7 +9,6 @@ if (as.logical(Sys.getenv("TEST_DATA_TABLE_WITH_OTHER_PACKAGES","FALSE"))) {
   # just results in NULL in other.Rout. Hence options(warn=1) because that
   # worked to display the warnings, not because we want them displayed at the
   # time per se.
-    
+
   test.data.table(script="other.Rraw")
 }
-
Original file line number	Diff line number	Diff line change
Expand Up		@@ -187,4 +187,3 @@ bmerge = function(i, x, icols, xcols, roll, rollends, nomatch, mult, ops, verbos
		ans$xo = xo # for further use by [.data.table
		return(ans)
		}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -118,4 +118,3 @@ uniqueN = function(x, by = if (is.list(x)) seq_along(x) else NULL, na.rm=FALSE)
		length(starts)
		}
		}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -247,4 +247,3 @@ foverlaps = function(x, y, by.x=if (!is.null(key(x))) key(x) else key(y), by.y=k
		# Tests are added to ensure we cover these aspects (to my knowledge) to ensure that any undesirable changes in the future breaks those tests.

		# Conclusion: floating point manipulations are hell!
Original file line number	Diff line number	Diff line change
Expand Up		@@ -13,4 +13,3 @@ setDTthreads = function(threads=NULL, restore_after_fork=NULL, percent=NULL, thr
		getDTthreads = function(verbose=getOption("datatable.verbose")) {
		.Call(CgetDTthreads, verbose)
		}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -271,4 +271,3 @@ trunc_cols_message = function(not_printed, abbs, class, col.names){
		n, brackify(paste0(not_printed, classes))
		)
		}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -364,4 +364,3 @@ CJ = function(..., sorted = TRUE, unique = FALSE)
		}
		l
		}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -290,4 +290,3 @@ all.equal.data.table = function(target, current, trim.levels=TRUE, check.attribu
		}
		TRUE
		}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -12,4 +12,3 @@ timetaken = function(started.at)
		tt = proc.time()-started.at # diff all 3 times
		paste0(format(tt[3L])," elapsed (", format(tt[1L]), " cpu)")
		}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -21,4 +21,3 @@ uniqlengths = function(x, len) {
		ans = .Call(Cuniqlengths, as.integer(x), as.integer(len))
		ans
		}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -165,4 +165,3 @@ rss = function() { #5515 #5517
		round(ans / 1024, 1L) # return MB
		# nocov end
		}