Skip to content

Commit d9517a9

Browse files
Handle backtickd names in glue extraction (#1630)
* Handle backtickd names in glue extraction * ignore str2lang usage * attempt workaround for str2lang * str2lang not a backports export * silence R CMD check issue again * bad merge Co-authored-by: Indrajeet Patil <[email protected]>
1 parent bf9d642 commit d9517a9

File tree

6 files changed

+41
-15
lines changed

6 files changed

+41
-15
lines changed

.lintr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
linters: linters_with_defaults(
22
line_length_linter(120),
33
implicit_integer_linter(),
4-
backport_linter("oldrel-4", except = "R_user_dir")
4+
backport_linter("oldrel-4", except = c("R_user_dir", "str2lang"))
55
)
66
exclusions: list(
77
"inst/doc/creating_linters.R" = 1,

.lintr_new

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
linters: linters_with_defaults(
22
any_duplicated_linter(),
33
any_is_na_linter(),
4-
backport_linter("oldrel-4", except = "R_user_dir"),
4+
backport_linter("oldrel-4", except = c("R_user_dir", "str2lang")),
55
consecutive_stopifnot_linter(),
66
expect_comparison_linter(),
77
expect_length_linter(),

NEWS.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,11 @@
1212
* Set the default for the `except` argument in `duplicate_argument_linter()` to `c("mutate", "transmute")`.
1313
This allows sequential updates like `x |> mutate(a = b + 1, a = log(a))` (#1345, @IndrajeetPatil).
1414

15-
* `object_usage_linter()` gains `skip_with` argument to skip code in `with()` expressions.
16-
To be consistent with `R CMD check`, it defaults to `TRUE` (#941, #1458, @IndrajeetPatil).
15+
* `object_usage_linter()`
16+
+ gains `skip_with` argument to skip code in `with()` expressions. To be consistent with
17+
`R CMD check`, it defaults to `TRUE` (#941, #1458, @IndrajeetPatil).
18+
+ Handles backticked symbols inside {glue} expressions correctly, e.g. ``glue("{`x`}")`` correctly
19+
determines `x` was used (#1619, @MichaelChirico)
1720

1821
* `spaces_inside_linter()` allows terminal missing keyword arguments (e.g. `alist(arg = )`; #540, @MichaelChirico)
1922

R/object_usage_linter.R

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -175,23 +175,19 @@ extract_glued_symbols <- function(expr) {
175175
if (length(glue_calls) == 0L) {
176176
return(character())
177177
}
178-
glued_symbols <- new.env(parent = emptyenv())
179178

180179
unexpected_error <- function(cond) {
181180
stop("Unexpected failure to parse glue call, please report: ", conditionMessage(cond)) # nocov
182181
}
183-
for (cl in glue_calls) {
182+
glued_symbols <- new.env(parent = emptyenv())
183+
for (call_text in xml2::xml_text(glue_calls)) {
184184
# TODO(michaelchirico): consider dropping tryCatch() here if we're more confident in our logic
185-
parsed_cl <- tryCatch(
186-
parse(text = xml2::xml_text(cl)),
187-
error = unexpected_error,
188-
warning = unexpected_error
189-
)[[1L]]
190-
parsed_cl[[".envir"]] <- glued_symbols
191-
parsed_cl[[".transformer"]] <- symbol_extractor
185+
parsed_call <- tryCatch(str2lang(call_text), error = unexpected_error, warning = unexpected_error)
186+
parsed_call[[".envir"]] <- glued_symbols
187+
parsed_call[[".transformer"]] <- symbol_extractor
192188
# #1459: syntax errors in glue'd code are ignored with warning, rather than crashing lint
193189
tryCatch(
194-
eval(parsed_cl),
190+
eval(parsed_call),
195191
error = function(cond) {
196192
warning(
197193
"Evaluating glue expression while testing for local variable usage failed: ",
@@ -215,7 +211,9 @@ symbol_extractor <- function(text, envir, data) {
215211
return("")
216212
}
217213
parse_data <- utils::getParseData(parsed_text)
218-
symbols <- parse_data$text[parse_data$token == "SYMBOL"]
214+
215+
# strip backticked symbols; `x` is the same as x.
216+
symbols <- gsub("^`(.*)`$", "\\1", parse_data$text[parse_data$token == "SYMBOL"])
219217
for (sym in symbols) {
220218
assign(sym, NULL, envir = envir)
221219
}

R/zzz.R

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,10 @@ settings <- NULL
285285

286286
# This is just here to quiet R CMD check
287287
if (FALSE) backports::import
288+
# requires R>=3.6.0; see https://github.com/r-lib/backports/issues/68
289+
if (!exists("str2lang", getNamespace("base"))) {
290+
assign("str2lang", get("str2lang", getNamespace("backports")), getNamespace(pkgname))
291+
}
288292

289293
default_settings <<- list(
290294
linters = default_linters,

tests/testthat/test-object_usage_linter.R

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,27 @@ test_that("errors/edge cases in glue syntax don't fail lint()", {
471471
)
472472
})
473473

474+
test_that("backtick'd names in glue are handled", {
475+
expect_lint(
476+
trim_some("
477+
fun <- function() {
478+
`w` <- 2
479+
x <- 3
480+
y <- -4
481+
`\\`y` <- 4
482+
z <- -5
483+
`z\\`` <- 5
484+
glue::glue('{w}{`x`}{y}{z}')
485+
}
486+
"),
487+
list(
488+
rex::rex("local variable '`y'"),
489+
rex::rex("local variable 'z`'")
490+
),
491+
object_usage_linter()
492+
)
493+
})
494+
474495
# reported as #1088
475496
test_that("definitions below top level are ignored (for now)", {
476497
expect_lint(

0 commit comments

Comments
 (0)