Validate config files

R/exclude.R

@@ -10,18 +10,21 @@
 #'   "@details",
 #'   "Exclusions can be specified in three different ways.",
 #'   "",
-#'   "1. single line in the source file. default: `# nolint`, possibly followed by a listing of linters to exclude.",
+#'   "1. Single line in the source file. default: `# nolint`, possibly followed by a listing of linters to exclude.",
 #'   "   If the listing is missing, all linters are excluded on that line. The default listing format is",
 #'   paste(
 #'     "   `#",
 #'     "nolint: linter_name, linter2_name.`. There may not be anything between the colon and the line exclusion tag"
 #'   ),
 #'   "   and the listing must be terminated with a full stop (`.`) for the linter list to be respected.",
-#'   "2. line range in the source file. default: `# nolint start`, `# nolint end`. `# nolint start` accepts linter",
+#'   "2. Line range in the source file. default: `# nolint start`, `# nolint end`. `# nolint start` accepts linter",
 #'   "   lists in the same form as `# nolint`.",
-#'   "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."
+#'   "3. Exclusions parameter, a list with named and/or unnamed entries. If present, the name is a path relative to ",
+#'   "   the config. Moreover, elements have the following characteristics:",
+#'   "   1. Unnamed elements specify filenames or directories.",
+#'   "   2. Named elements are numeric or a list. If present, the name is a linter.",
+#'   "      1. Numeric or unnamed list elements are line numbers to be excluded.",
+#'   "      2. Named list elements are line numbers to be excluded for specific linters."
 #' )
 exclude <- function(lints, exclusions = settings$exclusions, linter_names = NULL, ...) {
   if (length(lints) <= 0L) {

R/settings.R

@@ -32,6 +32,7 @@ read_settings <- function(filename) {
   }
 
   config <- read_config_file(config_file)
+  validate_config_file(config, config_file, default_settings)
 
   for (setting in names(default_settings)) {
     value <- get_setting(setting, config, default_settings)
@@ -77,6 +78,72 @@ read_config_file <- function(config_file) {
   config
 }
 
+validate_config_file <- function(config, config_file, defaults) {
+  matched <- names(config) %in% names(defaults)
+  if (!all(matched)) {
+    warning("Found non-setting objects in config '", config_file, "': ", toString(names(config)[!matched]))
+  }
+
+  check_types <- intersect(names(config), names(defaults))
+  for (setting in check_types) {
+    observed_type <- typeof(config[[setting]])
+    expected_type <- typeof(defaults[[setting]])
+    if (observed_type != expected_type) {
+      stop("Setting '", setting, "' should be of type '", expected_type, "', not '", observed_type, "'.")
+    }
+  }
+
+  validate_linters(config)
+  validate_exclusions(config)
+}
+
+validate_linters <- function(config) {
+  if (!"linters" %in% names(config)) {
+    return(invisible())
+  }
+
+  is_linter <- vapply(config$linters, inherits, "linter", FUN.VALUE = logical(1L))
+  if (!all(is_linter)) {
+    stop(
+      "Setting 'linters' should be a list of linters, but found non-linters at elements ",
+      toString(which(!is_linter)), "."
+    )
+  }
+}
+
+validate_exclusions <- function(config) {
+  if (!"exclusions" %in% names(config)) {
+    return(invisible())
+  }
+
+  config_names <- names2(config$exclusions)
+  has_names <- config_names != ""
+  unnamed_is_string <-
+    vapply(config$exclusions[!has_names], function(x) is.character(x) && length(x) == 1L && !is.na(x), logical(1L))
+  if (!all(unnamed_is_string)) {
+    stop(
+      "Unnamed entries of setting 'exclusions' should be strings naming files or directories, check entries: ",
+      toString(which(!has_names)[!unnamed_is_string]), "."
+    )
+  }
+  for (ii in which(has_names)) validate_named_exclusion(config, ii)
+}
+
+validate_named_exclusion <- function(config, idx) {
+  entry <- config$exclusions[[idx]]
+  if (is.list(entry)) {
+    valid_entry <- vapply(entry, function(x) is.numeric(x) && !anyNA(x), logical(1L))
+  } else {
+    valid_entry <- is.numeric(entry) && !anyNA(entry)
+  }
+  if (!all(valid_entry)) {
+    stop(
+      "Named entries of setting 'exclusions' should designate line numbers for exclusion, ",
+      "check exclusion: ", idx, "."
+    )
+  }
+}
+
 lintr_option <- function(setting, default = NULL) getOption(paste0("lintr.", setting), default)
 
 get_setting <- function(setting, config, defaults) {

R/zzz.R

@@ -245,7 +245,7 @@ default_undesirable_operators <- all_undesirable_operators[names(all_undesirable
 #'  - `exclude`: pattern used to exclude a line of code
 #'  - `exclude_start`, `exclude_end`: patterns used to mark start and end of the code block to exclude
 #'  - `exclude_linter`, `exclude_linter_sep`: patterns used to exclude linters
-#'  - `exclusions`:a list of files to exclude
+#'  - `exclusions`: a list of exclusions, see [exclude()] for a complete description of valid values.
 #'  - `cache_directory`: location of cache directory
 #'  - `comment_token`: a GitHub token character
 #'  - `comment_bot`: decides if lintr comment bot on GitHub can comment on commits

tests/testthat/test-lint_package.R

@@ -217,7 +217,11 @@ test_that("package using .lintr.R config lints correctly", {
 
   # config produces unused variables
   withr::local_options(lintr.linter_file = "lintr_test_config_extraneous")
-  expect_length(lint_package(r_config_pkg), 2L)
+  expect_warning(
+    expect_length(lint_package(r_config_pkg), 2L),
+    "Found non-setting objects in config",
+    fixed = TRUE
+  )
 
   # DCF is preferred if multiple matched configs
   withr::local_options(lintr.linter_file = "lintr_test_config_conflict")

tests/testthat/test-settings.R

@@ -152,3 +152,58 @@ test_that("it has a smart default for encodings", {
   lintr:::read_settings(pkg_file)
   expect_identical(settings$encoding, "ISO8859-1")
 })
+
+test_that("validate_config_file() detects improperly-formed settings", {
+  .lintr <- withr::local_tempfile()
+  withr::local_options(lintr.linter_file = .lintr)
+  withr::local_dir(withr::local_tempdir())
+
+  writeLines("asdf: 1", .lintr)
+  expect_warning(lint_dir(), "Found non-setting objects in config", fixed = TRUE)
+
+  writeLines("a=1", "aaa.R")
+  writeLines(c('exclusions: list("aaa.R")', "asdf: 1"), .lintr)
+  expect_warning(lint_dir(), "Found non-setting objects in config", fixed = TRUE)
+
+  writeLines("exclude: FALSE", .lintr)
+  expect_error(lint_dir(), "Setting 'exclude' should be of type 'character', not 'logical'", fixed = TRUE)
+
+  writeLines(c('exclusions: list("aaa.R")', "exclude: FALSE"), .lintr)
+  expect_error(lint_dir(), "Setting 'exclude' should be of type 'character', not 'logical'", fixed = TRUE)
+
+  writeLines("linters: list(1)", .lintr)
+  expect_error(lint_dir(), "Setting 'linters' should be a list of linters", fixed = TRUE)
+
+  writeLines("linters: list(assignment_linter(), 1)", .lintr)
+  expect_error(lint_dir(), "Setting 'linters' should be a list of linters", fixed = TRUE)
+
+  writeLines("exclusions: list(1L)", .lintr)
+  expect_error(lint_dir(), "Unnamed entries of setting 'exclusions' should be strings", fixed = TRUE)
+
+  writeLines('exclusions: list("aaa.R", 1L)', .lintr)
+  expect_error(lint_dir(), "Unnamed entries of setting 'exclusions' should be strings", fixed = TRUE)
+
+  writeLines("exclusions: list(letters)", .lintr)
+  expect_error(lint_dir(), "Unnamed entries of setting 'exclusions' should be strings", fixed = TRUE)
+
+  writeLines("exclusions: list(NA_character_)", .lintr)
+  expect_error(lint_dir(), "Unnamed entries of setting 'exclusions' should be strings", fixed = TRUE)
+
+  writeLines('exclusions: list(aaa.R = "abc")', .lintr)
+  expect_error(lint_dir(), "Named entries of setting 'exclusions' should designate line numbers", fixed = TRUE)
+
+  writeLines("exclusions: list(aaa.R = NA_integer_)", .lintr)
+  expect_error(lint_dir(), "Named entries of setting 'exclusions' should designate line numbers", fixed = TRUE)
+
+  writeLines('exclusions: list(aaa.R = list("abc"))', .lintr)
+  expect_error(lint_dir(), "Named entries of setting 'exclusions' should designate line numbers", fixed = TRUE)
+
+  writeLines("exclusions: list(aaa.R = list(NA_integer_))", .lintr)
+  expect_error(lint_dir(), "Named entries of setting 'exclusions' should designate line numbers", fixed = TRUE)
+
+  writeLines('exclusions: list(aaa.R = list(assignment_linter = "abc"))', .lintr)
+  expect_error(lint_dir(), "Named entries of setting 'exclusions' should designate line numbers", fixed = TRUE)
+
+  writeLines("exclusions: list(aaa.R = list(assignment_linter = NA_integer_))", .lintr)
+  expect_error(lint_dir(), "Named entries of setting 'exclusions' should designate line numbers", fixed = TRUE)
+})