Skip to content

Commit

Permalink
dont lint nested string literals on LHS of assignment (#2038)
Browse files Browse the repository at this point in the history
Co-authored-by: AshesITR <alexander.rosenstock@web.de>
  • Loading branch information
MichaelChirico and AshesITR authored Aug 4, 2023
1 parent 239de20 commit bb7669a
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 5 deletions.
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
20 changes: 15 additions & 5 deletions R/object_name_linter.R
Original file line number Diff line number Diff line change
@@ -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
")
})
Expand Down
6 changes: 6 additions & 0 deletions tests/testthat/test-object_name_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})

0 comments on commit bb7669a

Please sign in to comment.