Incorrect "no visible binding for global variable"

[bersbersbers]

This example code

foo <- data.frame(0)
foo$bar <- 1
zero <- function() {
    file.info("/dev/null")$size
}
message(zero())

lints with this error:

<text>:5:28: warning: no visible binding for global variable ‘size’
    file.info("/dev/null")$size
                           ^~~~

lintr::lint('
foo <- data.frame(0)
foo$bar <- 1
zero <- function() {
    file.info("/dev/null")$size
}
message(zero())
')

The error disappears when you take away the foo$bar <- 1:

lintr::lint('
foo <- data.frame(0)
zero <- function() {
    file.info("/dev/null")$size
}
message(zero())
')

[AshesITR]

Confirmed. The problem lies somewhere in the way object_usage_linter uses codetools::checkUsage().

[dmurdoch]

I think the problem lies in the get_assignment_symbols function: it returns c( "foo", "foo", "$", "bar", "zero") as left_assignment_symbols. If I edit that to return just c("foo", "zero"), I don't get the false positive.

Here's the line that produces the bad result:

left_assignment_symbols <-
    xml2::xml_text(xml2::xml_find_all(xml, "expr[LEFT_ASSIGN]/expr[1]/*"))

I think it needs a predicate saying that expr[1] is a SYMBOL, not an expression like foo$bar. Unfortunately, I don't know XML well enough to know how to write that.

[AshesITR]

@dmurdoch Thanks for the hint.
I've figured out an XPath that should always work for a$b$c$d-chains (returning a) as well as direct assignments:

expr[LEFT_ASSIGN]/expr[1]/*[1]/descendant-or-self::SYMBOL[1]

The *[1] makes sure we're inside the LHS of the assignment operator and descendant-or-self::SYMBOL[1] ensures we only take the first SYMBOL element per result.

[dmurdoch]

That appears to miss complex assignments like names(x) <- "y". But I don't understand the code well enough to know if this would be a problem.

[AshesITR]

Hmm I see. Maybe a better Idea would be to check that the parent expression is no DOLLAR-OP...

[russHyde]

The same bug occured in #445 (which I have closed to remove duplicate issues)

[AshesITR]

@russHyde do you mind taking a look at #709?
I'm not sure whether the changed XPath works for all possible assignments.

[russHyde]

Sure. I'm not an XPath expert by any means. I can compare object-usage over some packages before/after your changes if that's helpful.

[AshesITR]

That would be very useful indeed.
Seeing no observable side-effects would increase my confidence in the XPath-fu job.