Avoid implicit assignments in expectations (#1879)

* getting started

* use magrittr pipe instead

* handle a few more

* a few more

* and some more

* Update test-xml_nodes_to_lints.R

* fix indentation lints

* remove magrittr brace syntax in favour of tempvar

* don't gain dependency on magrittr

Author: IndrajeetPatil

tests/testthat/test-backport_linter.R

@@ -12,12 +12,6 @@ test_that("backport_linter detects backwards-incompatibility", {
   expect_lint(".getNamespaceInfo(dir.exists(lapply(x, toTitleCase)))", NULL, backport_linter("release"))
   expect_lint(".getNamespaceInfo(dir.exists(lapply(x, toTitleCase)))", NULL, backport_linter("devel"))
 
-  # don't allow dependencies older than we've recorded
-  tmp <- withr::local_tempfile(lines = "x <- x + 1")
-
-  expect_warning(l <- lint(tmp, backport_linter("2.0.0")), "version older than 3.0.0", fixed = TRUE)
-  expect_identical(l, lint(tmp, backport_linter("3.0.0")))
-
   expect_lint(
     "numToBits(2)",
     rex::rex("numToBits (R 4.1.0) is not available for dependency R >= 4.0.0."),
@@ -64,3 +58,16 @@ test_that("backport_linter detects backwards-incompatibility", {
     backport_linter("3.0.0", except = c("numToBits", "R_user_dir"))
   )
 })
+
+test_that("backport_linter generates expected warnings", {
+  tmp <- withr::local_tempfile(lines = "x <- x + 1")
+
+  expect_warning(
+    {
+      l <- lint(tmp, backport_linter("2.0.0"))
+    },
+    "version older than 3.0.0",
+    fixed = TRUE
+  )
+  expect_identical(l, lint(tmp, backport_linter("3.0.0")))
+})

tests/testthat/test-cache.R

@@ -92,10 +92,19 @@ test_that("load_cache returns an empty environment if reading cache file fails",
   cache_f1 <- file.path(d1, fhash(f1))
   writeLines(character(), cache_f1)
 
-  expect_warning(e2 <- lintr:::load_cache(file = f1, path = d1), "Could not load cache file")
+  expect_warning(
+    {
+      e2 <- lintr:::load_cache(file = f1, path = d1)
+    },
+    "Could not load cache file"
+  )
   saveRDS(e1, cache_f1)
-  expect_warning(e3 <- lintr:::load_cache(file = f1, path = d1), "Could not load cache file")
-
+  expect_warning(
+    {
+      e3 <- lintr:::load_cache(file = f1, path = d1)
+    },
+    "Could not load cache file"
+  )
   expect_identical(ls(e2), character())
   expect_identical(ls(e3), character())
 })

tests/testthat/test-closed_curly_linter.R

@@ -7,7 +7,9 @@ test_that("returns the correct linting", {
   )
 
   expect_warning(
-    linter <- closed_curly_linter(),
+    {
+      linter <- closed_curly_linter()
+    },
     "Linter closed_curly_linter was deprecated",
     fixed = TRUE
   )

tests/testthat/test-dir_linters.R

@@ -95,8 +95,11 @@ test_that("lint_dir works with specific linters without specifying other argumen
 
 test_that("lint_dir continues to accept relative_path= in 2nd positional argument, with a warning", {
   the_dir <- test_path("dummy_packages", "package", "vignettes")
+
   expect_warning(
-    positional_lints <- lint_dir(the_dir, FALSE),
+    {
+      positional_lints <- lint_dir(the_dir, FALSE)
+    },
     "'relative_path' is no longer available as a positional argument",
     fixed = TRUE
   )

tests/testthat/test-get_source_expressions.R

@@ -336,15 +336,16 @@ param_df <- expand.grid(
   expression_idx = seq_along(expressions),
   stringsAsFactors = FALSE
 )
-param_df$.test_name <-
-  with(param_df, sprintf("%s on expression %d", linter, expression_idx))
+param_df$.test_name <- with(param_df, sprintf("%s on expression %d", linter, expression_idx))
 
 patrick::with_parameters_test_that(
   "linters pass with xml_missing() content",
   {
     linter <- eval(call(linter))
     expression <- expressions[[expression_idx]]
-    expect_no_warning(lints <- linter(expression))
+    expect_no_warning({
+      lints <- linter(expression)
+    })
     expect_length(lints, 0L)
   },
   .test_name = param_df$.test_name,

tests/testthat/test-ids_with_token.R

@@ -6,7 +6,9 @@ test_that("ids_with_token works as expected", {
 
   # deprecated argument
   expect_warning(
-    old_arg <- ids_with_token(source_file = source_expression, value = "expr"),
+    {
+      old_arg <- ids_with_token(source_file = source_expression, value = "expr")
+    },
     "Argument source_file was deprecated"
   )
   expect_identical(old_arg, ref)

tests/testthat/test-lint_file.R

@@ -221,7 +221,9 @@ test_that("compatibility warnings work", {
 
 test_that("Deprecated positional usage of cache= works, with warning", {
   expect_warning(
-    l <- lint("a = 2\n", FALSE, linters = assignment_linter()),
+    {
+      l <- lint("a = 2\n", FALSE, linters = assignment_linter())
+    },
     "'cache' is no longer available as a positional argument",
     fixed = TRUE
   )

tests/testthat/test-lint_package.R

@@ -132,7 +132,13 @@ test_that(
 test_that("lint_package returns early if no package is found", {
   temp_pkg <- withr::local_tempdir("dir")
 
-  expect_warning(l <- lint_package(temp_pkg), "Didn't find any R package", fixed = TRUE)
+  expect_warning(
+    {
+      l <- lint_package(temp_pkg)
+    },
+    "Didn't find any R package",
+    fixed = TRUE
+  )
   expect_null(l)
 
   # ignore a folder named DESCRIPTION, #702

tests/testthat/test-methods.R

@@ -143,6 +143,8 @@ test_that("split.lint works as intended", {
 
 test_that("within.list is dispatched", {
   l <- lint(text = "a=1\nb=2", linters = infix_spaces_linter())
-  expect_silent(l <- lapply(l, within, line_number <- line_number + 1L))
+  expect_silent({
+    l <- lapply(l, within, line_number <- line_number + 1L)
+  })
   expect_identical(vapply(l, `[[`, integer(1L), "line_number"), 2L:3L)
 })

tests/testthat/test-with.R

@@ -68,15 +68,19 @@ test_that("can instantiate all linters without arguments", {
 test_that("with_defaults is supported with a deprecation warning", {
   defaults <- linters_with_defaults()
   expect_warning(
-    old_defaults <- with_defaults(),
+    {
+      old_defaults <- with_defaults()
+    },
     rex::rex("Use linters_with_defaults or modify_defaults instead.")
   )
   expect_identical(defaults, old_defaults)
 
   # linters_with_defaults only accepts `defaults = list()` to start from blank
   defaults <- linters_with_defaults(defaults = list(), no_tab_linter())
   expect_warning(
-    old_defaults <- with_defaults(default = NULL, no_tab_linter()),
+    {
+      old_defaults <- with_defaults(default = NULL, no_tab_linter())
+    },
     rex::rex("Use linters_with_defaults or modify_defaults instead.")
   )
   expect_identical(defaults, old_defaults)
@@ -93,16 +97,25 @@ test_that("modify_defaults works", {
 })
 
 test_that("linters_with_defaults(default = .) is supported with a deprecation warning", {
-  expect_warning(linters <- linters_with_defaults(default = list(), no_tab_linter()), "'default'")
+  expect_warning(
+    {
+      linters <- linters_with_defaults(default = list(), no_tab_linter())
+    },
+    "'default'"
+  )
   expect_named(linters, "no_tab_linter")
 
   # the same warning is not triggered in modify_defaults
-  expect_silent(linters <- modify_defaults(defaults = list(), default = list(), no_tab_linter()))
+  expect_silent({
+    linters <- modify_defaults(defaults = list(), default = list(), no_tab_linter())
+  })
   expect_named(linters, c("default", "no_tab_linter"))
 
   # if default= is explicitly provided alongside defaults=, assume that was intentional
   default <- Linter(function(.) list())
-  expect_silent(linters <- linters_with_defaults(defaults = list(), default = default))
+  expect_silent({
+    linters <- linters_with_defaults(defaults = list(), default = default)
+  })
   expect_named(linters, "default")
 })