diff --git a/R/object_usage_linter.R b/R/object_usage_linter.R index a54ed25d5..c5c350497 100644 --- a/R/object_usage_linter.R +++ b/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) } diff --git a/tests/testthat/dummy_packages/err_pkg/DESCRIPTION b/tests/testthat/dummy_packages/err_pkg/DESCRIPTION deleted file mode 100644 index dc00c0a6a..000000000 --- a/tests/testthat/dummy_packages/err_pkg/DESCRIPTION +++ /dev/null @@ -1,3 +0,0 @@ -Package: error.pkg -Version: 0.0.1 -Title: Package with syntax errors diff --git a/tests/testthat/dummy_packages/err_pkg/NAMESPACE b/tests/testthat/dummy_packages/err_pkg/NAMESPACE deleted file mode 100644 index 3fd3db002..000000000 --- a/tests/testthat/dummy_packages/err_pkg/NAMESPACE +++ /dev/null @@ -1,2 +0,0 @@ -export(foo) - diff --git a/tests/testthat/dummy_packages/err_pkg/R/glue_error.R b/tests/testthat/dummy_packages/err_pkg/R/glue_error.R deleted file mode 100644 index 6b06dcf11..000000000 --- a/tests/testthat/dummy_packages/err_pkg/R/glue_error.R +++ /dev/null @@ -1,6 +0,0 @@ -# Regression test for #1459: syntax errors in code get helpful failure -foo <- function() { - remote_groups <- 1 - message(glue::glue("Groups found: {glue::glue_collapse(purrr::map_chr(remote_groups$results, ~ .x$name), sep = \", \")")) -} - diff --git a/tests/testthat/test-lint_file.R b/tests/testthat/test-lint_file.R index a9d94ba04..2370d77ea 100644 --- a/tests/testthat/test-lint_file.R +++ b/tests/testthat/test-lint_file.R @@ -212,7 +212,7 @@ test_that("compatibility warnings work", { expect_error( lint("a <- 1\n", linters = "equals_na_linter"), - regexp = rex("Expected '", anything, "' to be a function of class 'linter'") + regexp = rex::rex("Expected '", anything, "' to be a function of class 'linter'") ) }) @@ -224,3 +224,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()), + rex::rex("Linter 'linter' failed in ", anything, basename(tmp_file), ": a broken linter") + ) + expect_error( + lint(tmp_file, list(broken_linter = linter())), + rex::rex("Linter 'broken_linter' failed in ", anything, basename(tmp_file), ": a broken linter") + ) +}) diff --git a/tests/testthat/test-lint_package.R b/tests/testthat/test-lint_package.R index b9ad4123b..0c68afd93 100644 --- a/tests/testthat/test-lint_package.R +++ b/tests/testthat/test-lint_package.R @@ -137,13 +137,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 '}'" - ) -}) diff --git a/tests/testthat/test-object_usage_linter.R b/tests/testthat/test-object_usage_linter.R index c8ba70523..a7c6663a0 100644 --- a/tests/testthat/test-object_usage_linter.R +++ b/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(