From 90d8f31c791adfb4cd3f7b0618613a9ec66c5a2a Mon Sep 17 00:00:00 2001
From: Michael Chirico <chiricom@google.com>
Date: Fri, 4 Aug 2023 06:45:21 +0000
Subject: [PATCH] dont lint nested string literals on LHS of assignment

---
 NEWS.md                                  |  1 +
 R/object_name_linter.R                   | 20 +++++++++++++++-----
 tests/testthat/test-object_name_linter.R |  6 ++++++
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index cbb36398e..08fae0b0e 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -15,6 +15,7 @@
 * `object_usage_linter()`:
   + assumes `glue()` is `glue::glue()` when `interpret_glue=TRUE` (#2032, @MichaelChirico).
   + finds function usages inside `glue()` calls to avoid false positives for "unused objects" (#2029, @MichaelChirico).
+* `object_name_linter()` no longer attempts to lint strings in function calls on the LHS of assignments (#1466, @MichaelChirico).
 
 # lintr 3.1.0
 
diff --git a/R/object_name_linter.R b/R/object_name_linter.R
index 10674136b..c36e66f0b 100644
--- a/R/object_name_linter.R
+++ b/R/object_name_linter.R
@@ -1,20 +1,30 @@
 object_name_xpath <- local({
-  xp_assignment_target <- paste0(
+  # search ancestor:: axis for assignments of symbols for
+  #   cases like a$b$c. We only try to lint 'a' since 'b'
+  #   and 'c' might be beyond the user's control to name.
+  #   the tree structure for 'a$b$c <- 1' has 'a'
+  #   at the 'bottom' of the <expr> list; it is distinguished
+  #   from 'b' and 'c' by not having '$' as a sibling.
+  # search parent:: axis for assignments of strings because
+  #   the complicated nested assignment available for symbols
+  #   is not possible for strings, though we do still have to
+  #   be aware of cases like 'a$"b" <- 1'.
+  xp_assignment_target_fmt <- paste0(
     "not(preceding-sibling::OP-DOLLAR)",
-    "and ancestor::expr[",
+    "and %1$s::expr[",
     " following-sibling::LEFT_ASSIGN",
     " or preceding-sibling::RIGHT_ASSIGN",
     " or following-sibling::EQ_ASSIGN",
     "]",
-    "and not(ancestor::expr[",
+    "and not(%1$s::expr[",
     " preceding-sibling::OP-LEFT-BRACKET",
     " or preceding-sibling::LBB",
     "])"
   )
 
   glue::glue("
-  //SYMBOL[ {xp_assignment_target} ]
-  |  //STR_CONST[ {xp_assignment_target} ]
+  //SYMBOL[ {sprintf(xp_assignment_target_fmt, 'ancestor')} ]
+  |  //STR_CONST[ {sprintf(xp_assignment_target_fmt, 'parent')} ]
   |  //SYMBOL_FORMALS
   ")
 })
diff --git a/tests/testthat/test-object_name_linter.R b/tests/testthat/test-object_name_linter.R
index 9c19acbe8..c80ce292b 100644
--- a/tests/testthat/test-object_name_linter.R
+++ b/tests/testthat/test-object_name_linter.R
@@ -246,3 +246,9 @@ test_that("object_name_linter supports custom regexes", {
     object_name_linter(regexes = c(a = "^a$", "^b$"))
   )
 })
+
+test_that("complex LHS of := doesn't cause false positive", {
+  # "_l" would be included under previous logic which tried ancestor::expr[ASSIGN] for STR_CONST,
+  #   but only parent::expr[ASSIGN] is needed for strings.
+  expect_lint('dplyr::mutate(df, !!paste0(v, "_l") := df$a * 2)', NULL, object_name_linter())
+})