Use partial matching for linter names in exclusion directives (#1138)

* use partial matching for linter names in exclusion directives

* document()

* document()

* add missing roxy

* implicit integers: exclude.R

* implicit integer: lint.R

* implicit integer: function_left_parentheses.R

* feedback

Co-authored-by: Michael Chirico <chiricom@google.com>

Author: AshesITR

NEWS.md

@@ -68,6 +68,7 @@
 * New syntax to exclude only selected linters from linting lines or passages. Use `# nolint: linter_name, linter2_name.`
   or `# nolint start: linter_name, linter2_name.` in source files or named lists of line numbers in `.lintr`.
   (#660, @AshesITR)
+  + Extended to allow for partial matching as long as the supplied prefix is unique (#872, @AshesITR)
 * Fixed `spaces_left_parentheses_linter` sporadically causing warnings (#654, #674, @AshesITR)
 * Fixed `line_length_linter` causing duplicate lints for lines containing multiple expressions (#681, #682, @AshesITR)
 * `line_length_linter` now places the source marker at the margin of the affected line to improve user experience during 

R/T_and_F_symbol_linter.R

@@ -7,7 +7,7 @@
 #'   [linters] for a complete list of linters available in lintr. \cr
 #'   <https://style.tidyverse.org/syntax.html#logical-vectors>
 #' @export
-T_and_F_symbol_linter <- function() { # nolint: object_name_linter.
+T_and_F_symbol_linter <- function() { # nolint: object_name.
   Linter(function(source_expression) {
     if (is.null(source_expression$xml_parsed_content)) return(list())
 

R/exclude.R

@@ -2,6 +2,7 @@
 #'
 #' @param lints that need to be filtered.
 #' @param exclusions manually specified exclusions
+#' @param linter_names character vector of names of the active linters, used for parsing inline exclusions.
 #' @param ... additional arguments passed to [parse_exclusions()]
 #' @details
 #' Exclusions can be specified in three different ways.
@@ -15,15 +16,15 @@
 #'  3. exclusions parameter, a named list of files with named lists of linters and lines to exclude them on, a named
 #'     list of the files and lines to exclude, or just the filenames if you want to exclude the entire file, or the
 #'     directory names if you want to exclude all files in a directory.
-exclude <- function(lints, exclusions = settings$exclusions, ...) {
-  if (length(lints) <= 0) {
+exclude <- function(lints, exclusions = settings$exclusions, linter_names = NULL, ...) {
+  if (length(lints) <= 0L) {
     return(lints)
   }
 
   df <- as.data.frame(lints)
 
   filenames <- unique(df$filename)
-  source_exclusions <- lapply(filenames, parse_exclusions, ...)
+  source_exclusions <- lapply(filenames, parse_exclusions, linter_names = linter_names, ...)
   names(source_exclusions) <- filenames
 
 
@@ -58,9 +59,9 @@ line_info <- function(line_numbers, type = c("start", "end")) {
   type <- match.arg(type)
   range_word <- paste0("range ", type, if (length(line_numbers) != 1L) "s")
   n <- length(line_numbers)
-  if (n == 0) {
+  if (n == 0L) {
     paste("0", range_word)
-  } else if (n == 1) {
+  } else if (n == 1L) {
     paste0("1 ", range_word, " (line ", line_numbers, ")")
   } else {
     paste0(n, " ", range_word, " (lines ", toString(line_numbers), ")")
@@ -77,14 +78,16 @@ line_info <- function(line_numbers, type = c("start", "end")) {
 #' `exclude` or `exclude_start` marker.
 #' @param exclude_linter_sep regular expression used to split a linter list into indivdual linter names for exclusion.
 #' @param lines a character vector of the content lines of `file`
+#' @param linter_names Names of active linters
 #'
 #' @return A possibly named list of excluded lines, possibly for specific linters.
 parse_exclusions <- function(file, exclude = settings$exclude,
                              exclude_start = settings$exclude_start,
                              exclude_end = settings$exclude_end,
                              exclude_linter = settings$exclude_linter,
                              exclude_linter_sep = settings$exclude_linter_sep,
-                             lines = NULL) {
+                             lines = NULL,
+                             linter_names = NULL) {
   if (is.null(lines)) {
     lines <- read_lines(file)
   }
@@ -102,7 +105,7 @@ parse_exclusions <- function(file, exclude = settings$exclude,
   starts <- which(!is.na(start_locations))
   ends <- which(!is.na(end_locations))
 
-  if (length(starts) > 0) {
+  if (length(starts) > 0L) {
     if (length(starts) != length(ends)) {
       starts_msg <- line_info(starts, type = "start")
       ends_msg <- line_info(ends, type = "end")
@@ -114,7 +117,7 @@ parse_exclusions <- function(file, exclude = settings$exclude,
       linters_string <- substring(lines[starts[i]], start_locations[starts[i]])
       linters_string <- rex::re_matches(linters_string, exclude_linter)[, 1L]
 
-      exclusions <- add_exclusions(exclusions, excluded_lines, linters_string, exclude_linter_sep)
+      exclusions <- add_exclusions(exclusions, excluded_lines, linters_string, exclude_linter_sep, linter_names)
     }
   }
 
@@ -126,7 +129,7 @@ parse_exclusions <- function(file, exclude = settings$exclude,
   for (i in seq_along(nolints)) {
     linters_string <- substring(lines[nolints[i]], nolint_locations[nolints[i]])
     linters_string <- rex::re_matches(linters_string, exclude_linter)[, 1L]
-    exclusions <- add_exclusions(exclusions, nolints[i], linters_string, exclude_linter_sep)
+    exclusions <- add_exclusions(exclusions, nolints[i], linters_string, exclude_linter_sep, linter_names)
   }
 
   exclusions[] <- lapply(exclusions, function(lines) sort(unique(lines)))
@@ -154,13 +157,26 @@ add_excluded_lines <- function(exclusions, excluded_lines, excluded_linters) {
   exclusions
 }
 
-add_exclusions <- function(exclusions, lines, linters_string, exclude_linter_sep) {
+add_exclusions <- function(exclusions, lines, linters_string, exclude_linter_sep, linter_names) {
   # No match for linter list: Add to global excludes
   if (is.na(linters_string)) {
     exclusions <- add_excluded_lines(exclusions, lines, "")
   } else {
     # Matched a linter list: only add excluded lines for the listed linters.
     excluded_linters <- strsplit(linters_string, exclude_linter_sep)[[1L]]
+    if (!is.null(linter_names)) {
+      idxs <- pmatch(excluded_linters, linter_names, duplicates.ok = TRUE)
+      matched <- !is.na(idxs)
+      if (!all(matched)) {
+        bad <- excluded_linters[!matched]
+        warning(
+          "Could not find linter", if (length(bad) > 1L) "s" else "", " named ",
+          glue::glue_collapse(sQuote(bad), sep = ", ", last = " and "),
+          " in the list of active linters. Make sure the linter is uniquely identified by the given name or prefix."
+        )
+      }
+      excluded_linters[matched] <- linter_names[idxs[matched]]
+    }
     exclusions <- add_excluded_lines(exclusions, lines, excluded_linters)
   }
   exclusions
@@ -192,7 +208,7 @@ add_exclusions <- function(exclusions, lines, linters_string, exclude_linter_sep
 normalize_exclusions <- function(x, normalize_path = TRUE,
                                  root = getwd(),
                                  pattern = NULL) {
-  if (is.null(x) || length(x) <= 0) {
+  if (is.null(x) || length(x) <= 0L) {
     return(list())
   }
 

R/function_left_parentheses.R

@@ -7,7 +7,7 @@
 #'   [linters] for a complete list of linters available in lintr. \cr
 #'   <https://style.tidyverse.org/syntax.html#parentheses>
 #' @export
-function_left_parentheses_linter <- function() { # nolint: object_length_linter.
+function_left_parentheses_linter <- function() { # nolint: object_length.
   Linter(function(source_expression) {
     lapply(
       ids_with_token(source_expression, "'('"),
@@ -20,7 +20,7 @@ function_left_parentheses_linter <- function() { # nolint: object_length_linter.
             source_expression$parsed_content$col1 < parsed$col1 &
             source_expression$parsed_content$terminal
 
-        last_type <- tail(source_expression$parsed_content$token[terminal_tokens_before], n = 1)
+        last_type <- tail(source_expression$parsed_content$token[terminal_tokens_before], n = 1L)
 
         is_function_call <- length(last_type) %!=% 0L &&
           (last_type %in% c("SYMBOL_FUNCTION_CALL", "FUNCTION", "'}'", "']'"))

R/lint.R

@@ -104,7 +104,7 @@ lint <- function(filename, linters = NULL, ..., cache = FALSE, parse_settings =
     if (!is.null(lints)) {
       # TODO: once cache= is fully deprecated as 3rd positional argument (see top of body), we can restore the cleaner:
       # > exclude(lints, lines = lines, ...)
-      return(do.call(exclude, c(list(lints, lines = lines), dots)))
+      return(do.call(exclude, c(list(lints, lines = lines, linter_names = names(linters)), dots)))
     }
     cache <- TRUE
   } else {
@@ -152,7 +152,7 @@ lint <- function(filename, linters = NULL, ..., cache = FALSE, parse_settings =
 
   # TODO: once cache= is fully deprecated as 3rd positional argument (see top of body), we can restore the cleaner:
   # > exclude(lints, lines = lines, ...)
-  res <- do.call(exclude, c(list(lints, lines = lines), dots))
+  res <- do.call(exclude, c(list(lints, lines = lines, linter_names = names(linters)), dots))
 
   # simplify filename if inline
   if (no_filename) {
@@ -461,7 +461,7 @@ pkg_name <- function(path = find_package()) {
   if (is.null(path)) {
     return(NULL)
   } else {
-    read.dcf(file.path(path, "DESCRIPTION"), fields = "Package")[1]
+    read.dcf(file.path(path, "DESCRIPTION"), fields = "Package")[1L]
   }
 }
 
@@ -476,7 +476,7 @@ pkg_name <- function(path = find_package()) {
 #' @param linter deprecated. No longer used.
 #' @return an object of class 'lint'.
 #' @export
-Lint <- function(filename, line_number = 1L, column_number = 1L, # nolint: object_name_linter.
+Lint <- function(filename, line_number = 1L, column_number = 1L, # nolint: object_name.
                  type = c("style", "warning", "error"),
                  message = "", line = "", ranges = NULL, linter = "") {
   if (!missing(linter)) {

R/methods.R

@@ -137,7 +137,7 @@ split.lints <- function(x, f = NULL, ...) {
 }
 
 #' @export
-as.data.frame.lints <- function(x, row.names = NULL, optional = FALSE, ...) { # nolint: object_name_linter. (row.names)
+as.data.frame.lints <- function(x, row.names = NULL, optional = FALSE, ...) { # nolint: object_name. (row.names)
   data.frame(filename = vapply(x, `[[`, character(1), "filename"),
              line_number = vapply(x, `[[`, numeric(1), "line_number"),
              column_number = vapply(x, `[[`, numeric(1), "column_number"),

R/utils.R

@@ -177,7 +177,7 @@ reset_lang <- function(old_lang) {
 #' Lints produced by the linter will be labelled with `name` by default.
 #' @return The same function with its class set to 'linter'.
 #' @export
-Linter <- function(fun, name = linter_auto_name()) { # nolint: object_name_linter.
+Linter <- function(fun, name = linter_auto_name()) { # nolint: object_name.
   if (!is.function(fun) || length(formals(args(fun))) != 1L) {
     stop("`fun` must be a function taking exactly one argument.", call. = FALSE)
   }

man/exclude.Rd

@@ -0,0 +1,13 @@
+# nolint start: assign.
+a = 2
+# nolint end
+
+# nolint start: s. warn (and lint) because of non-unique identifier
+x <- 42; y <- 2
+# nolint end
+
+# nolint start: bogus_linter. warn because of nonexistent identifier
+
+# nolint end
+
+# nolint: hocus_pocus, bogus.

man/parse_exclusions.Rd

@@ -13,11 +13,13 @@ test_that("line_info works as expected", {
   )
 })
 
-old_ops <- options(lintr.exclude = "#TeSt_NoLiNt",
-                   lintr.exclude_start = "#TeSt_NoLiNt_StArT",
-                   lintr.exclude_end = "#TeSt_NoLiNt_EnD")
-
 test_that("it excludes properly", {
+  withr::local_options(
+    lintr.exclude = "#TeSt_NoLiNt",
+    lintr.exclude_start = "#TeSt_NoLiNt_StArT",
+    lintr.exclude_end = "#TeSt_NoLiNt_EnD"
+  )
+
   read_settings(NULL)
 
   t1 <- lint("exclusions-test", parse_settings = FALSE)
@@ -42,13 +44,16 @@ test_that("it excludes properly", {
 })
 
 test_that("it doesn't fail when encountering misspecified encodings", {
+  withr::local_options(
+    lintr.exclude = "#TeSt_NoLiNt",
+    lintr.exclude_start = "#TeSt_NoLiNt_StArT",
+    lintr.exclude_end = "#TeSt_NoLiNt_EnD"
+  )
   read_settings(NULL)
 
   expect_length(parse_exclusions("dummy_projects/project/cp1252.R"), 0L)
 })
 
-options(old_ops)
-
 test_that("it gives the expected error message when there is only one start but no end", {
   read_settings(NULL)
 
@@ -68,3 +73,22 @@ test_that("it gives the expected error message when there is mismatch between mu
     fixed = TRUE
   )
 })
+
+test_that("partial matching works for exclusions but warns if no linter found", {
+  read_settings(NULL)
+
+  expect_warning(
+    expect_warning(
+      expect_warning(
+        expect_lint(
+          file = "dummy_projects/project/partially_matched_exclusions.R",
+          checks = rex::rex("semicolons"),
+          parse_settings = FALSE
+        ),
+        rex::rex("Could not find linter named ", anything, "s")
+      ),
+      rex::rex("Could not find linter named ", anything, "bogus_linter")
+    ),
+    rex::rex("Could not find linters named ", anything, "hocus_pocus", anything, "bogus")
+  )
+})

tests/testthat/dummy_projects/project/partially_matched_exclusions.R

@@ -265,6 +265,25 @@ lintr::lint(
 )
 ```
 
+You can also specify the linter names by a unique prefix instead of their full name:
+
+**file5.R**
+```r
+X = 42L # nolint: object_name, line_len. this comment still overflows the default 80 chars line length.
+```
+
+`> lint("file5.R")`
+```{r, echo = FALSE}
+lintr::lint(
+  paste(
+    "X = 42L",
+    "# nolint: object_name, line_len. this comment still overflows the default 80 chars line length.\n"
+  ),
+  parse_settings = FALSE
+)
+```
+
+
 ### Excluding multiple lines of codes
 
 If any or all linters should be disabled for a contiguous block of code, the `exclude_start` and `exclude_end` patterns