Comment in glue() range causes parse failure in object_usage_linter

[MichaelChirico]

lint("
foo <- function() {
  x <- 2
  glue::glue(
    '{x}' # a comment
  )
}
", object_usage_linter())
# Error: Linter 'object_usage_linter' failed in /tmp/RtmpAirXij/file39151e5be6f486: Unexpected failure to parse glue call, please report: <text>:2:0: unexpected end of input
# 1: glue::glue('{x}'# a comment)
#    ^

[MichaelChirico]

The trouble is here:

lintr/R/object_usage_linter.R

Line 191 in c69f792

for (call_text in xml2::xml_text(glue_calls)) {

xml_text() will collapse all the text across the nodes, ignoring newlines, hence trying to parse glue::glue('{x}'#a comment).

What we'd ideally do is exclude any COMMENT nodes below each glue_call before attempting to parse.

@jimhester could you offer some guidance/xml2 wizardry here? At a glance xml_remove() is what we want, but we don't want to permanently modify the parse tree, is there an easy way to do that? An alternative would be to set xml_text() of the COMMENT nodes to '', then re-set it on.exit(), which feels a bit clunky too.

Related again to r-lib/xml2#341 (comment) :)

[jimhester]

I would use something like this to get just the text in the STR_CONST nodes, rather than any node type.

xml2::xml_text(xml2::xml_find_first(glue_calls, "//STR_CONST/text()"))

[AshesITR]

I would use something like this to get just the text in the STR_CONST nodes, rather than any node type.

xml2::xml_text(xml2::xml_find_first(glue_calls, "//STR_CONST/text()"))

This will break glue calls with multiple fragments, e.g.

glue::glue(
  "This is the first glued line with 1 + 1 = {1 + 1}\n",
  "This is the second line with pi = {pi}"
)

[MichaelChirico]

and a find_all approach will also have to ignore keyword arguments.

I think it's doable but feels like a workaround /much messier than glue_calls |> xml_exclude(comments) |> xml_text()

[AshesITR]

Do you think it might be feasible to instead develop a xml_parse_data_to_code(xml_node) function that uses the col1 and line1 attributes of nodes to construct the original code?
I can imagine we will have multiple use cases for such a function.

[MichaelChirico]

xml2lang() :)

not a bad idea, this thing with comments definitely comes up here & there

[jimhester]

paste0(collapse = "", xml2::xml_text(xml2::xml_find_all(glue_calls, "./*[not(self::COMMENT)]")))

[MichaelChirico]

ah, great minds :)

xml <- parse(text = "glue(\n'{x}' # comment\n)") |>
  xml_parse_data() |>
  read_xml()

xml_node2lang <- function(x) {
  x |>
    xml_find_all(".//*[not(self::COMMENT or self::expr)]") |>
    xml_text() |>
    paste(collapse = "") |>
    str2lang()
}
if (inherits(xml, "xml_document")) {
  # not xml_children, since xml_node2lang() will fail for comments: str2lang("")
  lapply(xml_find_all(xml, "*[not(self::COMMENT)]"), xml_node2lang)
} else {
  xml_node2lang(xml)
}

./* will struggle with more complicated expressions I think:

glue(
  format(
    Sys.Date(),
    "%F {x}" # YYYY-MM-DD, then 'x'
  )
)