Skip to content

Commit a7d3679

Browse files
Skip rlang name injections (#2545)
* Skip rlang name injections * revert * typo
1 parent 66085d0 commit a7d3679

File tree

4 files changed

+28
-3
lines changed

4 files changed

+28
-3
lines changed

NEWS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@
7272
* `one_call_pipe_linter()` for discouraging one-step pipelines like `x |> as.character()` (#2330 and part of #884, @MichaelChirico).
7373
* `object_overwrite_linter()` for discouraging re-use of upstream package exports as local variables (#2344, #2346 and part of #884, @MichaelChirico and @AshesITR).
7474

75+
### Lint accuracy fixes: removing false positives
76+
77+
* `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.
78+
7579
# lintr 3.1.2
7680

7781
## New and improved features

R/shared_constants.R

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ object_name_xpath <- local({
203203
xp_assignment_target_fmt <- paste0(
204204
"not(parent::expr[OP-DOLLAR or OP-AT])",
205205
"and %1$s::expr[",
206-
" following-sibling::LEFT_ASSIGN",
206+
" following-sibling::LEFT_ASSIGN%2$s",
207207
" or preceding-sibling::RIGHT_ASSIGN",
208208
" or following-sibling::EQ_ASSIGN",
209209
"]",
@@ -213,9 +213,15 @@ object_name_xpath <- local({
213213
"])"
214214
)
215215

216+
# strings on LHS of := are only checked if they look like data.table usage DT[, "a" := ...]
217+
dt_walrus_cond <- "[
218+
text() != ':='
219+
or parent::expr/preceding-sibling::OP-LEFT-BRACKET
220+
]"
221+
216222
glue("
217-
//SYMBOL[ {sprintf(xp_assignment_target_fmt, 'ancestor')} ]
218-
| //STR_CONST[ {sprintf(xp_assignment_target_fmt, 'parent')} ]
223+
//SYMBOL[ {sprintf(xp_assignment_target_fmt, 'ancestor', '')} ]
224+
| //STR_CONST[ {sprintf(xp_assignment_target_fmt, 'parent', dt_walrus_cond)} ]
219225
| //SYMBOL_FORMALS
220226
")
221227
})

tests/testthat/test-object_length_linter.R

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,3 +74,10 @@ test_that("function shorthand is caught", {
7474
object_length_linter(length = 10L)
7575
)
7676
})
77+
78+
test_that("rlang name injection is handled", {
79+
linter <- object_length_linter(length = 10L)
80+
81+
expect_lint("tibble('{foo() |> bar() |> baz()}' := TRUE)", NULL, linter)
82+
expect_lint("DT[, 'a_very_long_name' := FALSE]", "names should not be longer than 10 characters", linter)
83+
})

tests/testthat/test-object_name_linter.R

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,3 +278,11 @@ test_that("capture groups in style are fine", {
278278
expect_lint("a <- 1\nab <- 2", NULL, object_name_linter(regexes = c(capture = "^(a)")))
279279
expect_lint("ab <- 1\nabc <- 2", NULL, object_name_linter(regexes = c(capture = "^(a)(b)")))
280280
})
281+
282+
test_that("rlang name injection is handled", {
283+
linter <- object_name_linter()
284+
285+
expect_lint('tibble("{name}" := 2)', NULL, linter)
286+
expect_lint('x %>% mutate("{name}" := 2)', NULL, linter)
287+
expect_lint('DT[, "{name}" := 2]', "style should match snake_case", linter)
288+
})

0 commit comments

Comments
 (0)