[MichaelChirico]

Closes #750. Co-written with Gemini.

[MichaelChirico]

Alternatives Considered

1. Hooking into cli_*() / glue_cmd() via a .transformer argument or global option (similar to how {lintr} evaluates glue::glue() calls with a custom .transformer):
   • In {cli}, glue_cmd() is called from 20+ entry points (cli_abort(), cli_text(), cli_ul(), cli_dl(), cli_progress_bar(), etc.), many of which have side effects (signaling conditions, starting progress bars, mutating app state). A standalone function operating on character vectors avoids evaluating calls with side effects.
2. Stripping {cli} markup to return a {glue}-compatible string:
   • {cli} has already parsed each {...} expression inside make_cmd_transformer(), so re-serializing to a string only to re-parse with {glue} would duplicate work and lose {cli}'s C-level glue_(.cli = TRUE) delimiter/quoting semantics.
3. Returning raw expression objects instead of symbol names:
   • Returning character() via base::all.vars() directly satisfies the primary static-analysis use case while exposing functions and unique for caller control.

[MichaelChirico]

This now includes #848, for now, to get CI passing.

GitHub doesn't allow PR stacking from fork to upstream, but you can check the 'dummy' stack on my fork:

MichaelChirico#1

[gaborcsardi]

Thanks! Can you undo the changes that are not related to your PR?

[MichaelChirico]

Thanks! Can you undo the changes that are not related to your PR?

done, rebased

[gaborcsardi]

In general I am a bit worried about promising that this function will work well, especially that I am planning on rewriting the string interpolation in cli. We can add it, I understand that it'll be an improvement, but maybe catch errors in lintr and maybe suggest catching errors in the cli_extract_vars() docs.

Relatedly, are these expected?

> cli_extract_vars("{?s} {a} and {b}")
Error in post_process_plurals(pstr, values) : 
  Multiple quantities for pluralization

> cli_extract_vars("bad {?s}")
Error in post_process_plurals(pstr, values) : 
  Cannot pluralize without a quantity

> cli_extract_vars("{vapply(x, function(i) i + 1, 1)}")
[1] "vapply"   "x"        "function" "+"        "i"       

[MichaelChirico]

especially that I am planning on rewriting the string interpolation in cli

If that's an imminent thing, I'd be happy to defer this if you think it'll be more straightforward to do ex post.

For your examples:

---

cli_extract_vars("{?s} {a} and {b}") / cli_extract_vars("bad {?s}")

That looks like correct behavior to me, what else could we do? a=1;b=2;cli_abort("{?s} {a} {b}") gives the same error, where cli_abort("{a}{?s} {b}{?s}") might have been intended.

Gemini does suggest:

since {?...} directives only contain literal text alternatives and never R expressions, we could easily skip parse_plural() / post_process_plurals() when .eval = FALSE in make_cmd_transformer() and glue_cmd() if we want cli_extract_vars() to still extract c("a", "b") from "{?s} {a} and {b}" instead of failing on pluralization rules.

I'm not familiar enough with {cli} to judge if that's overkill.

---

cli_extract_vars("{vapply(x, function(i) i + 1, 1)}")

That comes straight from all.vars(functions=TRUE):

all.vars(quote(vapply(x, function(i) i + 1, 1)), functions = TRUE)
# [1] "vapply"   "x"        "function" "+"        "i"

If we were going to try to better than that we'd have to custom-roll an all.vars() analogue, probably not worth it.

In lintr::object_usage_linter(), the extracted symbols are only passed to codetools::checkUsage(..., suppressLocalUnused = known_used_symbols) as an allowlist to suppress false-positive "local variable assigned but may not be used" warnings—so extra tokens like "function", "+", or "i" are harmless unless the outer function also has a genuinely unused local variable named i.

---

maybe catch errors in lintr and maybe suggest catching errors

Definitely; 811e565 aims to facilitate that by making sure {cli} validation errors are properly classed

[gaborcsardi]

Thanks!