Skip to content

Commit bb7669a

Browse files
dont lint nested string literals on LHS of assignment (#2038)
Co-authored-by: AshesITR <alexander.rosenstock@web.de>
1 parent 239de20 commit bb7669a

File tree

3 files changed

+22
-5
lines changed

3 files changed

+22
-5
lines changed

NEWS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
* `object_usage_linter()`:
1616
+ assumes `glue()` is `glue::glue()` when `interpret_glue=TRUE` (#2032, @MichaelChirico).
1717
+ finds function usages inside `glue()` calls to avoid false positives for "unused objects" (#2029, @MichaelChirico).
18+
* `object_name_linter()` no longer attempts to lint strings in function calls on the LHS of assignments (#1466, @MichaelChirico).
1819

1920
# lintr 3.1.0
2021

R/object_name_linter.R

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,30 @@
11
object_name_xpath <- local({
2-
xp_assignment_target <- paste0(
2+
# search ancestor:: axis for assignments of symbols for
3+
# cases like a$b$c. We only try to lint 'a' since 'b'
4+
# and 'c' might be beyond the user's control to name.
5+
# the tree structure for 'a$b$c <- 1' has 'a'
6+
# at the 'bottom' of the <expr> list; it is distinguished
7+
# from 'b' and 'c' by not having '$' as a sibling.
8+
# search parent:: axis for assignments of strings because
9+
# the complicated nested assignment available for symbols
10+
# is not possible for strings, though we do still have to
11+
# be aware of cases like 'a$"b" <- 1'.
12+
xp_assignment_target_fmt <- paste0(
313
"not(preceding-sibling::OP-DOLLAR)",
4-
"and ancestor::expr[",
14+
"and %1$s::expr[",
515
" following-sibling::LEFT_ASSIGN",
616
" or preceding-sibling::RIGHT_ASSIGN",
717
" or following-sibling::EQ_ASSIGN",
818
"]",
9-
"and not(ancestor::expr[",
19+
"and not(%1$s::expr[",
1020
" preceding-sibling::OP-LEFT-BRACKET",
1121
" or preceding-sibling::LBB",
1222
"])"
1323
)
1424

1525
glue::glue("
16-
//SYMBOL[ {xp_assignment_target} ]
17-
| //STR_CONST[ {xp_assignment_target} ]
26+
//SYMBOL[ {sprintf(xp_assignment_target_fmt, 'ancestor')} ]
27+
| //STR_CONST[ {sprintf(xp_assignment_target_fmt, 'parent')} ]
1828
| //SYMBOL_FORMALS
1929
")
2030
})

tests/testthat/test-object_name_linter.R

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,3 +246,9 @@ test_that("object_name_linter supports custom regexes", {
246246
object_name_linter(regexes = c(a = "^a$", "^b$"))
247247
)
248248
})
249+
250+
test_that("complex LHS of := doesn't cause false positive", {
251+
# "_l" would be included under previous logic which tried ancestor::expr[ASSIGN] for STR_CONST,
252+
# but only parent::expr[ASSIGN] is needed for strings.
253+
expect_lint('dplyr::mutate(df, !!paste0(v, "_l") := df$a * 2)', NULL, object_name_linter())
254+
})

0 commit comments

Comments
 (0)