Find some = or -> assignments when discovering script globals (#2656)

Co-authored-by: AshesITR <[email protected]>

Author: MichaelChirico

NEWS.md

@@ -86,6 +86,7 @@
 ### Lint accuracy fixes: removing false positives
 
 * `object_name_linter()` and `object_length_linter()` ignore {rlang} name injection like `x |> mutate("{new_name}" := foo(col))` (#1926, @MichaelChirico). No checking is applied in such cases. {data.table} in-place assignments like `DT[, "sPoNGeBob" := "friend"]` are still eligible for lints.
+* `object_usage_linter()` finds global variables assigned with `=` or `->`, which avoids some issues around "undefined global variables" in scripts (#2654, @MichaelChirico).
 
 ## Notes
 

R/object_usage_linter.R

@@ -148,8 +148,10 @@ get_assignment_symbols <- function(xml) {
   get_r_string(xml_find_all(
     xml,
     "
-      expr[LEFT_ASSIGN]/expr[1]/SYMBOL[1] |
+      expr[LEFT_ASSIGN or EQ_ASSIGN]/expr[1]/SYMBOL[1] |
+      expr[RIGHT_ASSIGN]/expr[2]/SYMBOL[1] |
       equal_assign/expr[1]/SYMBOL[1] |
+      expr_or_assign_or_help/expr[1]/SYMBOL[1] |
       expr[expr[1][SYMBOL_FUNCTION_CALL/text()='assign']]/expr[2]/* |
       expr[expr[1][SYMBOL_FUNCTION_CALL/text()='setMethod']]/expr[2]/*
     "

tests/testthat/test-object_usage_linter.R

@@ -748,6 +748,9 @@ test_that("symbols in formulas aren't treated as 'undefined global'", {
 })
 
 test_that("NSE-ish symbols after $/@ are ignored as sources for lints", {
+  linter <- object_usage_linter()
+  lint_msg <- "no visible binding for global variable 'column'"
+
   expect_lint(
     trim_some("
       foo <- function(x) {
@@ -757,12 +760,8 @@ test_that("NSE-ish symbols after $/@ are ignored as sources for lints", {
         )
       }
     "),
-    list(
-      message = "no visible binding for global variable 'column'",
-      line_number = 4L,
-      column_number = 22L
-    ),
-    object_usage_linter()
+    list(lint_msg, line_number = 4L, column_number = 22L),
+    linter
   )
 
   expect_lint(
@@ -774,12 +773,8 @@ test_that("NSE-ish symbols after $/@ are ignored as sources for lints", {
         )
       }
     "),
-    list(
-      message = "no visible binding for global variable 'column'",
-      line_number = 4L,
-      column_number = 22L
-    ),
-    object_usage_linter()
+    list(lint_msg, line_number = 4L, column_number = 22L),
+    linter
   )
 })
 
@@ -798,53 +793,34 @@ test_that("functional lambda definitions are also caught", {
 })
 
 test_that("messages without location info are repaired", {
+  linter <- object_usage_linter()
+  global_function_msg <- rex::rex("no visible global function definition for", anything)
+  global_variable_msg <- rex::rex("no visible binding for global variable", anything)
+  local_variable_msg <- rex::rex("local variable", anything, "assigned but may not be used")
+
   # regression test for #1986
   expect_lint(
-    trim_some("
-      foo <- function() no_fun()
-    "),
-    list(
-      message = rex::rex("no visible global function definition for", anything),
-      line_number = 1L,
-      column_number = 19L
-    ),
-    object_usage_linter()
+    "foo <- function() no_fun()",
+    list(global_function_msg, line_number = 1L, column_number = 19L),
+    linter
   )
 
   expect_lint(
-    trim_some("
-      foo <- function(a = no_fun()) a
-    "),
-    list(
-      message = rex::rex("no visible global function definition for", anything),
-      line_number = 1L,
-      column_number = 21L
-    ),
-    object_usage_linter()
+    "foo <- function(a = no_fun()) a",
+    list(global_function_msg, line_number = 1L, column_number = 21L),
+    linter
   )
 
   expect_lint(
-    trim_some("
-      foo <- function() no_global
-    "),
-    list(
-      message = rex::rex("no visible binding for global variable", anything),
-      line_number = 1L,
-      column_number = 19L
-    ),
-    object_usage_linter()
+    "foo <- function() no_global",
+    list(global_variable_msg, line_number = 1L, column_number = 19L),
+    linter
   )
 
   expect_lint(
-    trim_some("
-      foo <- function() unused_local <- 42L
-    "),
-    list(
-      message = rex::rex("local variable", anything, "assigned but may not be used"),
-      line_number = 1L,
-      column_number = 19L
-    ),
-    object_usage_linter()
+    "foo <- function() unused_local <- 42L",
+    list(local_variable_msg, line_number = 1L, column_number = 19L),
+    linter
   )
 
   # More complex case with two lints and missing location info
@@ -854,17 +830,31 @@ test_that("messages without location info are repaired", {
         bar()
     "),
     list(
-      list(
-        message = rex::rex("local variable", anything, "assigned but may not be used"),
-        line_number = 1L,
-        column_number = 19L
-      ),
-      list(
-        message = rex::rex("no visible global function definition for", anything),
-        line_number = 2L,
-        column_number = 3L
-      )
+      list(local_variable_msg, line_number = 1L, column_number = 19L),
+      list(global_function_msg, line_number = 2L, column_number = 3L)
     ),
-    object_usage_linter()
+    linter
+  )
+})
+
+test_that("globals in scripts are found regardless of assignment operator", {
+  linter <- object_usage_linter()
+
+  expect_lint(
+    trim_some("
+      library(dplyr)
+
+      global_const_eq = 5
+      global_const_la <- 6
+      7 -> global_const_ra
+
+      examplefunction <- function(df) {
+        df %>%
+          select(dist) %>%
+          mutate(power = global_const_eq + global_const_ra + global_const_la)
+      }
+    "),
+    NULL,
+    linter
   )
 })