lintr icon indicating copy to clipboard operation
lintr copied to clipboard

Linter for checking if variable used has definition in scope?

Open jmburgos opened this issue 4 years ago • 7 comments

I noticed that in RStudio a syntax error in a function name (say priint instead of print) will raise a "No symbol named priint in scope" warning. I would have to have the same warning pop un in emacs (where I am using ess & flymake), but I cannot figure out which linter provides it. With the default linters in ess & flymake, a syntax error like this does not get flagged. Thanks for the help!

jmburgos avatar Dec 12 '19 13:12 jmburgos

Although it should be included in the default linters already, could you try using the object_usage_linter and report back, please.

russHyde avatar Dec 13 '19 11:12 russHyde

Thanks Russ. I do not get the syntax error message. To make sure it is not an emacs isse, I ran R in the console. I did "lintr(test.R)", where test.R contained a single line of code. If the code was priint ("hello") I only get the following:

/home/julian/Documents/org_files/dotcode.R:1:8: style: Remove spaces before the left parenthesis in a function call.
priint ("hello")
       ^

So I do not get any warning about the syntax error. When I removed the space before the parenthesis, lintr does not find any problem in the file.

jmburgos avatar Dec 16 '19 09:12 jmburgos

Right. Yeah I've just reproduced this:

# temp.R
priint("hello")

f <- function(x) {
  priint(x)
}

g <- function(x) {
  some_package::priint(x)
}
R> library(lintr)
R> lint("temp.R", linters = list(object_usage_linter))

This only flags up the "no visible global function definition for ‘priint’" within the definition for f.

At heart, I think this boils down to the use of codetools::checkUsage (within object_usage_linter), which only checks object usage within function-definitions (so it ignores the top-level priint call & I guess it just assumes that some_package exports an appropriately-named object, since the package some_package doesn't exist).

lintr does find invalid R syntax (but your priint example has valid syntax - if your script was sourced by a file that defined priint, it would run fine), so things like

invalid R code

9.invalid_objectName <- 123

would be caught.


For standalone scripts, a fix could be something like: For a given .R script:

  • find all objects used in that script
  • for each object (used at top-level), check whether it is defined in the preceding code or is exported by one of the base or attached packages (or the transitively attached packages)
  • for anything that isn't defined at point of usage, throw a lint

But this is problematic for packages (within function A you can use function B, even if B is defined below A in the defining script; and you can use objects that are defined in other files) and for scripts that will be called in some context that ensures the objects are available (eg, if your script was sourced by another script, or if it sourced a script prior to the priint line)

So I think the issue you've raised is a valid one, but perhaps not simple to fix.

russHyde avatar Dec 16 '19 10:12 russHyde

I understand. I assume then that the syntax warning that you get in RStudio is not coming from lintr but from some other syntax checking tool, right?

jmburgos avatar Dec 16 '19 10:12 jmburgos

I'd have to defer to @jimhester (since I don't know anything about Rstudio internals TBH)

russHyde avatar Dec 16 '19 10:12 russHyde

Yes RStudio does not use lintr, it has a custom linter written in C++.

jimhester avatar Dec 16 '19 14:12 jimhester

Internally, we use codetools::checkUsage() which requires functions to work. What could be done, though I'm not sure of performance implications, is run codetools::checkUsage() on an artifically created anonymous function, i.e. create an anonymous function whose body() is the complete file source code:

codetools::checkUsage(as.function(c(alist(), quote(priint ("hello")))))
#> <anonymous>: no visible global function definition for ‘priint’

Where quote(...) would be replaced by parse(text = file_lines).

AshesITR avatar Jun 19 '22 15:06 AshesITR