Add ignore_order= argument to expect_lint()

NEWS.md

@@ -23,6 +23,7 @@
 * `indentation_linter()` handles `for` un-braced for loops correctly (#2564, @MichaelChirico).
 * Setting `exclusions` supports globs like `knitr*` to exclude files/directories with a pattern (#1554, @MichaelChirico).
 * `object_name_linter()` and `object_length_linter()` apply to objects assigned with `assign()` or generics created with `setGeneric()` (#1665, @MichaelChirico).
+* `expect_lint()` has a new argument `ignore_order` (default `FALSE`), which, if `TRUE`, allows the `checks=` to be provided in arbitary order vs. how `lint()` produces them (@MichaelChirico).
 
 ### Lint accuracy fixes: removing false positives
 

R/expect_lint.R

@@ -4,9 +4,9 @@
 #' * `expect_lint` asserts that specified lints are generated.
 #' * `expect_no_lint` asserts that no lints are generated.
 #'
-#' @param content a character vector for the file content to be linted, each vector element representing a line of
-#' text.
-#' @param checks checks to be performed:
+#' @param content A character vector for the file content to be linted, each vector element representing a line of
+#'   text.
+#' @param checks Checks to be performed:
 #' \describe{
 #'   \item{NULL}{check that no lints are returned.}
 #'   \item{single string or regex object}{check that the single lint returned has a matching message.}
@@ -16,11 +16,14 @@
 #'     corresponding named list (as described in the point above).}
 #' }
 #' Named vectors are also accepted instead of named lists, but this is a compatibility feature that
-#' is not recommended for new code.
-#' @param ... arguments passed to [lint()], e.g. the linters or cache to use.
-#' @param file if not `NULL`, read content from the specified file rather than from `content`.
-#' @param language temporarily override Rs `LANGUAGE` envvar, controlling localization of base R error messages.
-#' This makes testing them reproducible on all systems irrespective of their native R language setting.
+#'   is not recommended for new code.
+#' @param ... Arguments passed to [lint()], e.g. the linters or cache to use.
+#' @param file If not `NULL`, read content from the specified file rather than from `content`.
+#' @param language Temporarily override Rs `LANGUAGE` envvar, controlling localization of base R error messages.
+#'   This makes testing them reproducible on all systems irrespective of their native R language setting.
+#' @param ignore_order Logical, default `FALSE`. If `TRUE`, the order of the `checks` does not matter, e.g.
+#'   lints with higher line numbers can come before those with lower line numbers, and the order of linters
+#'   affecting the same line is also irrelevant.
 #' @return `NULL`, invisibly.
 #' @examples
 #' # no expected lint
@@ -41,7 +44,7 @@
 #'   trailing_blank_lines_linter()
 #' )
 #' @export
-expect_lint <- function(content, checks, ..., file = NULL, language = "en") {
+expect_lint <- function(content, checks, ..., file = NULL, language = "en", ignore_order = FALSE) {
   require_testthat()
 
   old_lang <- set_lang(language)
@@ -59,11 +62,11 @@ expect_lint <- function(content, checks, ..., file = NULL, language = "en") {
 
   lints <- lint(file, ...)
   n_lints <- length(lints)
-  lint_str <- if (n_lints) paste(c("", lints), collapse = "\n") else ""
+  lint_fmt <- function() if (n_lints > 0L) paste(c("", lints), collapse = "\n") else ""
 
   wrong_number_fmt <- "got %d lints instead of %d%s"
   if (is.null(checks)) {
-    msg <- sprintf(wrong_number_fmt, n_lints, length(checks), lint_str)
+    msg <- sprintf(wrong_number_fmt, n_lints, length(checks), lint_fmt())
     return(testthat::expect(n_lints %==% 0L, msg))
   }
 
@@ -73,14 +76,26 @@ expect_lint <- function(content, checks, ..., file = NULL, language = "en") {
   checks[] <- lapply(checks, fix_names, "message")
 
   if (n_lints != length(checks)) {
-    msg <- sprintf(wrong_number_fmt, n_lints, length(checks), lint_str)
+    msg <- sprintf(wrong_number_fmt, n_lints, length(checks), lint_fmt())
     return(testthat::expect(FALSE, msg))
   }
 
+  if (ignore_order) {
+    lint_order <- with(as.data.frame(lints), order(line_number, column_number, linter))
+    lints <- lints[lint_order]
+
+    check_order <- order(
+      vapply(checks, function(x) x$line_number %||% 0L, FUN.VALUE = integer(1L)),
+      vapply(checks, function(x) x$column_number %||% 0L, FUN.VALUE = integer(1L)),
+      vapply(checks, function(x) x$linter %||% "", FUN.VALUE = character(1L))
+    )
+    checks <- checks[check_order]
+  }
+
   local({
     itr <- 0L
-    # keep 'linter' as a field even if we remove the deprecated argument from Lint() in the future
-    lint_fields <- unique(c(names(formals(Lint)), "linter"))
+    # valid fields are those from Lint(), plus 'linter'
+    lint_fields <- c(names(formals(Lint)), "linter")
     Map(
       function(lint, check) {
         itr <<- itr + 1L

tests/testthat/test-expect_lint.R

@@ -84,3 +84,27 @@ test_that("execution without testthat gives the right errors", {
   expect_error(expect_no_lint(), lint_msg("expect_no_lint"))
   expect_error(expect_lint_free(), lint_msg("expect_lint_free"))
 })
+
+test_that("lint order can be ignored", {
+  linters <- list(assignment_linter(), infix_spaces_linter())
+  expected <- lapply(linters, function(l) list(linter = attr(l, "name")))
+  expect_success(expect_lint("a=1", expected, linters, ignore_order = TRUE))
+  expect_success(expect_lint("a=1", rev(expected), linters, ignore_order = TRUE))
+
+  lines <- trim_some("
+    a=1
+    b=2
+  ")
+  expected <- list(list(line_number = 1L), list(line_number = 2L))
+  expect_success(expect_lint(lines, expected, assignment_linter(), ignore_order = TRUE))
+  expect_success(expect_lint(lines, rev(expected), assignment_linter(), ignore_order = TRUE))
+
+  # a fuzz test, since base R doesn't have a trivial way to do permutations
+  expected <- list(
+    list(linter = "assignment_linter", line_number = 1L),
+    list(linter = "assignment_linter", line_number = 2L),
+    list(linter = "infix_spaces_linter", line_number = 1L),
+    list(linter = "infix_spaces_linter", line_number = 2L)
+  )
+  expect_success(expect_lint(lines, expected[sample.int(4L)], linters, ignore_order = TRUE))
+})