Improve resilience of object_usage_linter() to glue syntax issues

R/object_usage_linter.R

@@ -183,7 +183,18 @@ extract_glued_symbols <- function(expr) {
       }
       ""
     }
-    eval(parsed_cl)
+    # #1459: syntax errors in glue'd code are ignored with warning, rather than crashing lint
+    tryCatch(
+      eval(parsed_cl),
+      error = function(cond) {
+        warning(
+          "Evaluating glue expression while testing for local variable usage failed: ",
+          conditionMessage(cond),
+          call. = FALSE
+        )
+        NULL
+      }
+    )
   }
   ls(envir = glued_symbols, all.names = TRUE)
 }

tests/testthat/test-lint_file.R

@@ -225,3 +225,18 @@ test_that("Deprecated positional usage of cache= works, with warning", {
   )
   expect_identical(l, lint("a = 2\n", assignment_linter(), cache = FALSE))
 })
+
+test_that("Linters throwing an error give a helpful error", {
+  tmp_file <- withr::local_tempfile(lines = "a <- 1")
+  linter <- function() Linter(function(source_expression) stop("a broken linter"))
+  # NB: Some systems/setups may use e.g. symlinked files when creating under tempfile();
+  #   we don't care much about that, so just check basename()
+  expect_error(
+    lint(tmp_file, linter()),
+    paste0("Linter 'linter' failed in .*", basename(tmp_file), ": a broken linter")
+  )
+  expect_error(
+    lint(tmp_file, list(broken_linter = linter())),
+    paste0("Linter 'broken_linter' failed in .*", basename(tmp_file), ": a broken linter")
+  )
+})

tests/testthat/test-lint_package.R

@@ -134,13 +134,3 @@ test_that("lint_package returns early if no package is found", {
     fixed = TRUE
   )
 })
-
-test_that("lint_package gives helpful failure when package has syntax issues", {
-  pkg_path <- test_path("dummy_packages", "err_pkg")
-  withr::local_options(lintr.linter_file = "lintr_test_config")
-
-  expect_error(
-    lint_package(pkg_path, linters = list(object_usage_linter(), assignment_linter())),
-    "Linter 'object_usage_linter' failed in.*glue_error.R: Expecting '}'"
-  )
-})

tests/testthat/test-object_usage_linter.R

@@ -393,6 +393,39 @@ test_that("interprets glue expressions", {
   "), "local_var", object_usage_linter(interpret_glue = FALSE))
 })
 
+test_that("errors in glue syntax don't fail lint()", {
+  # no lint & no error, despite glue error
+  expect_warning(
+    expect_lint(
+      trim_some("
+        fun <- function() {
+          a <- 2
+          a + 1
+          glue::glue('The answer is {a')
+        }
+      "),
+      NULL,
+      object_usage_linter()
+    ),
+    "Evaluating glue expression.*failed: Expecting '\\}'"
+  )
+
+  # generates a lint because the "usage" inside glue() is not detected
+  expect_warning(
+    expect_lint(
+      trim_some("
+        fun <- function() {
+          a <- 2
+          glue::glue('The answer is {a')
+        }
+      "),
+      "local variable 'a'",
+      object_usage_linter()
+    ),
+    "Evaluating glue expression.*failed: Expecting '\\}'"
+  )
+})
+
 # reported as #1088
 test_that("definitions below top level are ignored (for now)", {
   expect_lint(