Suppress coercion warnings (#2568)

NEWS.md

@@ -22,6 +22,7 @@
 * `.lintr` config validation correctly accepts regular expressions which only compile under `perl = TRUE` (#2375, @MichaelChirico). These have always been valid (since `rex::re_matches()`, which powers the lint exclusion logic, also uses this setting), but the new up-front validation in v3.1.1 incorrectly used `perl = FALSE`.
 * `.lintr` configs set by option `lintr.linter_file` or environment variable `R_LINTR_LINTER_FILE` can point to subdirectories (#2512, @MichaelChirico).
 * `indentation_linter()` returns `ranges[1L]==1L` when the offending line has 0 spaces (#2550, @MichaelChirico).
+* `literal_coercion_linter()` doesn't surface a warning about NAs during coercion for code like `as.integer("a")` (#2566, @MichaelChirico).
 
 ## Changes to default linters
 

R/literal_coercion_linter.R

@@ -97,9 +97,14 @@ literal_coercion_linter <- function() {
  needs_prefix <- is_rlang_coercer & !startsWith(coercion_str, "rlang::")
  coercion_str[needs_prefix] <- paste0("rlang::", coercion_str[needs_prefix])
  }
- # the linter logic & rlang requirement should ensure that it's safe to run eval() here
+ # the linter logic & rlang requirement should ensure that it's safe to run eval() here;
+ # suppressWarnings() is for cases like 'as.integer("a")' which have an NA result, #2566.
  # TODO(#2473): Avoid a recommendation like '1' that clashes with implicit_integer_linter().
- literal_equivalent_str <- vapply(str2expression(coercion_str), function(expr) deparse1(eval(expr)), character(1L))
+ literal_equivalent_str <- vapply(
+ str2expression(coercion_str),
+ function(expr) deparse1(suppressWarnings(eval(expr))),
+ character(1L)
+ )
  lint_message <- sprintf(
  "Use %s instead of %s, i.e., use literals directly where possible, instead of coercion.",
  literal_equivalent_str, report_str

tests/testthat/test-literal_coercion_linter.R

@@ -1,5 +1,5 @@
 test_that("literal_coercion_linter skips allowed usages", {
- linter <- line_length_linter()
+ linter <- literal_coercion_linter()
 
  # naive xpath includes the "_f0" here as a literal
  expect_lint('as.numeric(x$"_f0")', NULL, linter)
@@ -23,7 +23,7 @@ test_that("literal_coercion_linter skips allowed usages", {
 })
 
 test_that("literal_coercion_linter skips allowed rlang usages", {
- linter <- line_length_linter()
+ linter <- literal_coercion_linter()
 
  expect_lint("int(1, 2.0, 3)", NULL, linter)
  expect_lint("chr('e', 'ab', 'xyz')", NULL, linter)
@@ -40,6 +40,18 @@ test_that("literal_coercion_linter skips quoted keyword arguments", {
  expect_lint("as.numeric(foo('a' = 1))", NULL, literal_coercion_linter())
 })
 
+test_that("no warnings surfaced by running coercion", {
+ linter <- literal_coercion_linter()
+
+ expect_no_warning(
+ expect_lint("as.integer('a')", "Use NA_integer_", linter)
+ )
+
+ expect_no_warning(
+ expect_lint("as.integer(2147483648)", "Use NA_integer_", linter)
+ )
+})
+
 skip_if_not_installed("tibble")
 patrick::with_parameters_test_that(
  "literal_coercion_linter blocks simple disallowed usages",