Unify the linters a bit (#7077)

* Introduce and document .ci/lint.R

For use in both .github/workflows/code-quality.yaml and manually,
especially when developing the linters.

* code-quality: switch lint-c, lint-po to .ci/lint.R
* code-quality: switch lint-md to .ci/lint.R
* lint-md: make sure to use Rscript

* Update .ci/lint.R

Use str2lang().

Co-authored-by: Michael Chirico <[email protected]>

* Update .ci/linters/md/heading_id_linter.R

Co-authored-by: Michael Chirico <[email protected]>

* Update .ci/linters/md/news_linter.R

Co-authored-by: Michael Chirico <[email protected]>

* lint.R: move preprocessing to a special function

The preprocessing functions were too unwieldy to leave them on the
command line. This makes it possible to run any lint check directly from
the command line before committing code.

Co-authored-by: Michael Chirico <[email protected]>

---------

Co-authored-by: Michael Chirico <[email protected]>

Author: aitap

.ci/README.md

@@ -50,6 +50,23 @@ Base R implemented helper script, [originally proposed to base R](https://svn.r-
 
 Base R implemented helper script to orchestrate generation of most artifacts and to arrange them nicely. It is being used only in [_integration_ stage in GitLab CI pipeline](./../.gitlab-ci.yml).
 
+### [`lint.R`](./lint.R)
+
+Base R runner for the manual (non-`lintr`) lint checks to be run from GitHub Actions during the code quality check. The command line arguments are as follows:
+1. Path to the directory containing files defining the linters. A linter is a function that accepts one argument (typically the path to the file) and signals an error if it fails the lint check.
+2. Path to the directory containing files to check.
+3. A regular expression matching the files to check.
+
+One of the files in the linter directory may define the `.preprocess` function, which must accept one file path and return a value that other linter functions will understand. The function may also return `NULL` to indicate that the file must be skipped.
+
+Example command lines:
+
+```sh
+Rscript .ci/lint.R .ci/linters/c src '[.][ch]$'
+Rscript .ci/lint.R .ci/linters/po po '[.]po$'
+Rscript .ci/lint.R .ci/linters/md . '[.]R?md$'
+```
+
 ## GitLab Open Source Program
 
 We are currently part of the [GitLab for Open Source Program](https://about.gitlab.com/solutions/open-source/). This gives us 50,000 compute minutes per month for our GitLab CI. Our license needs to be renewed yearly (around July) and is currently managed by @ben-schwen.

.ci/lint.R

@@ -0,0 +1,36 @@
+#!/usr/bin/Rscript
+# Runner for the manual lint checks in .ci/linters
+args = commandArgs(TRUE)
+if (identical(args, '--help')) {
+  writeLines(c(
+    'Usage: Rscript .ci/lint.R .ci/linters/<KIND> <WHERE> <WHAT> [PREPROCESS]',
+    'KIND must name the directory containing the *.R files defining the linter functions.',
+    'WHERE must name the directory containing the files to lint, e.g. "po", or "src".',
+    "WHAT must contain the regular expression matching the files to lint, e.g., '[.]po$', or '[.][ch]$'.",
+  ))
+  q('no')
+}
+stopifnot(`Invalid arguments, see .ci/lint.R --help` = length(args) == 3)
+
+linter_env = list2env(list(.preprocess = identity))
+for (f in list.files(args[[1]], full.names=TRUE)) sys.source(f, linter_env)
+if (!length(ls(linter_env))) stop(
+  "No linters found after sourcing files in ", dQuote(args[[1]])
+)
+
+sources = list.files(args[[2]], pattern = args[[3]], full.names = TRUE, recursive = TRUE)
+if (!length(sources)) stop(
+  "No files to lint found in directory ", dQuote(args[[2]]), " for mask ", dQuote(args[[3]])
+)
+sources = Filter(Negate(is.null), lapply(setNames(nm = sources), linter_env$.preprocess))
+
+okay = TRUE
+for (src in names(sources))
+for (linter in ls(linter_env)) tryCatch(
+  linter_env[[linter]](sources[[src]]),
+  error = function(e) {
+    message('Source file ', dQuote(src), ' failed lint check ', dQuote(linter), ': ', conditionMessage(e))
+    okay <<- FALSE
+  }
+)
+stopifnot(`Please fix the issues above.` = okay)

.ci/linters/c/00preprocess.R

@@ -0,0 +1,7 @@
+.preprocess = function (f) list(
+  c_obj = f, lines = readLines(f),
+  preprocessed = system2(
+    "gcc", shQuote(c("-fpreprocessed", "-E", f)),
+    stdout = TRUE, stderr = FALSE
+  )
+)

.ci/linters/md/heading_id_linter.R

@@ -1,8 +1,8 @@
-any_mismatch = FALSE
-
 # ensure that ids are limited to alphanumerics and dashes
 # (in particular, dots and underscores break the links)
 check_header_ids = function(md) {
+  if (!grepl('[.]Rmd$', md)) return(invisible())
+  md = readLines(md)
   # A bit surprisingly, some headings don't start with a letter.
   # We're interested in those that set an id to link to, i.e., end with {#id}.
   heading_captures = regmatches(md, regexec("^#+ \\S.*[{]#([^}]*)[}]$", md))
@@ -14,13 +14,5 @@ check_header_ids = function(md) {
     "On line %d, bad heading id '%s':\n%s\n",
     line, heading_captures[[line]][2], heading_captures[[line]][1]
   ))
-  !all(good_ids)
+  stopifnot('Please fix the vignette issues above' = all(good_ids))
 }
-
-any_error = FALSE
-for (vignette in list.files('vignettes', pattern = "[.]Rmd$", recursive = TRUE, full.name = TRUE)) {
-  cat(sprintf("Checking vignette file %s...\n", vignette))
-  rmd_lines = readLines(vignette)
-  any_error = check_header_ids(rmd_lines) || any_error
-}
-if (any_error) stop("Please fix the vignette issues above.")

.ci/linters/md/news_linter.R

@@ -1,13 +1,13 @@
-any_mismatch = FALSE
-
 # ensure that numbered list in each section is in sequence
 check_section_numbering = function(news) {
+  if (!grepl("NEWS", news)) return(invisible())
+  news = readLines(news)
   # plain '#' catches some examples; 'd' for 'data.table'
   sections = grep("^#+ [A-Zd]", news)
   entries = grep("^[0-9]+[.]", news)
   entry_value = as.integer(gsub("^([0-9]+)[.].*", "\\1", news[entries]))
   section_id = findInterval(entries, sections)
-  
+
   any_mismatch = FALSE
   for (id in unique(section_id)) {
     section_entries = entry_value[section_id == id]
@@ -22,11 +22,12 @@ check_section_numbering = function(news) {
       paste0("  [", section_entries[!matched], " --> ", intended_value[!matched], "]", collapse="\n")
     ))
   }
-  return(any_mismatch)
+  stopifnot("Please fix the NEWS issues above" = !any_mismatch)
 }
 
 # ensure that GitHub link text & URL actually agree
 check_gh_links = function(news) {
+  news = readLines(news)
   gh_links_info = gregexpr(
     "\\[#(?<md_number>[0-9]+)\\]\\(https://github.com/Rdatatable/data.table/(?<link_type>[^/]+)/(?<link_number>[0-9]+)\\)",
     news,
@@ -48,14 +49,5 @@ check_gh_links = function(news) {
     "In line %d, link pointing to %s %s is written #%s\n",
     line_number, link_type, link_number, md_number
   )))
-  return(TRUE)
-}
-
-any_error = FALSE
-for (news in list.files(pattern = "NEWS")) {
-  cat(sprintf("Checking NEWS file %s...\n", news))
-  news_lines = readLines(news)
-  any_error = check_section_numbering(news_lines) || any_error
-  any_error = check_gh_links(news_lines) || any_error
+  stop("Please fix the NEWS issues above.")
 }
-if (any_error) stop("Please fix the NEWS issues above.")

.ci/linters/po/00preprocess.R

@@ -0,0 +1,4 @@
+.preprocess = function (f) {
+  diff_v_master = system2('git', c('diff', 'master', f), stdout=TRUE)
+  if (length(diff_v_master)) f
+}

.github/workflows/code-quality.yaml

@@ -40,40 +40,24 @@ jobs:
         run: /usr/bin/sudo apt-get -y install coccinelle
       - name: Lint
         run: |
-          linter_env = new.env()
-          for (f in list.files('.ci/linters/c', full.names=TRUE)) sys.source(f, linter_env)
-          for (f in list.files('src', pattern='[.][ch]$', full.names=TRUE)) {
-            c_obj = list(path = f, lines = readLines(f))
-            c_obj$preprocessed = system2("gcc", c("-fpreprocessed", "-E", f), stdout=TRUE, stderr=FALSE)
-            for (linter in ls(linter_env)) linter_env[[linter]](c_obj)
-            # TODO(#6272): Incorporate more checks from CRAN_Release
-          }
-        shell: Rscript {0}
+          Rscript .ci/lint.R .ci/linters/c src '[.][ch]$'
   lint-po:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
       - uses: r-lib/actions/setup-r@v2
       - name: Check translations
+        # only pay attention to files edited in the current PR, otherwise we can get
+        # a situation like after #6424 where some untranslated messages were added
+        # as part of non-translation maintenance, but this GHA would go red repeatedly
+        # until a translation is added or the blank/fuzzy translations removed. We'd
+        # rather only have the failure on one PR, then ignore these files later.
         run: |
-          linter_env = new.env()
-          for (f in list.files('.ci/linters/po', full.names=TRUE)) sys.source(f, linter_env)
-          for (po_file in list.files(pattern = "[.]po$", full.names=TRUE)) {
-            # only pay attention to files edited in the current PR, otherwise we can get
-            #   a situation like after #6424 where some untranslated messages were added
-            #   as part of non-translation maintenance, but this GHA would go red repeatedly
-            #   until a translation is added or the blank/fuzzy translations removed. We'd
-            #   rather only have the failure on one PR, then ignore these files later.
-            diff_v_master = system2("git", c("diff", "master", po_file), stdout=TRUE)
-            if (!length(diff_v_master)) next
-            for (linter in ls(linter_env)) linter_env[[linter]](po_file)
-          }
-        shell: Rscript {0}
+          Rscript .ci/lint.R .ci/linters/po po '[.]po$'
   lint-md:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
       - uses: r-lib/actions/setup-r@v2
       - name: Lint
-        run: for (f in list.files('.ci/linters/md', full.names=TRUE)) source(f)
-        shell: Rscript {0}
+        run: Rscript .ci/lint.R .ci/linters/md . '[.]R?md$'