r-lib / lintr

Static Code Analysis for R
https://lintr.r-lib.org
Other
1.19k stars 184 forks source link

New warning in 'designer' from object_usage_linter #1986

Closed MichaelChirico closed 1 year ago

MichaelChirico commented 1 year ago

Haven't confirmed this is a regression yet, but we observe this warning on main that is not present on v3.0.2:

Possible bug in lintr: Couldn't parse usage message ‘<anonymous>: no visible global function definition for 'find_cache_dir'
’. Ignoring 1 usage warnings. Please report an issue at https://github.com/r-lib/lintr/issues.
MichaelChirico commented 1 year ago

Similar warnings also in other packages:

** latrend
Possible bug in lintr: Couldn't parse usage message ‘<anonymous>: no visible binding for global variable 'id'
’. Ignoring 1 usage warnings. Please report an issue at https://github.com/r-lib/lintr/issues.
** mlrCPO
Possible bug in lintr: Couldn't parse usage message ‘<anonymous>: local variable 't' assigned but may not be used
’. Ignoring 1 usage warnings. Please report an issue at https://github.com/r-lib/lintr/issues.
** rextendr
Possible bug in lintr: Couldn't parse usage message ‘<anonymous>: no visible binding for global variable 'wrap__hello_world'
’. Ignoring 1 usage warnings. Please report an issue at https://github.com/r-lib/lintr/issues.
MichaelChirico commented 1 year ago

Here's an MRE for the type of error that's unrecognized:

lintr::lint(linters = lintr::object_usage_linter(), "
foo <- function() bar()
")
Warning message:
In parse_check_usage(fun, known_used_symbols = known_used_symbols,  :
  Possible bug in lintr: Couldn't parse usage message ‘<anonymous>: no visible global function definition for 'bar'
’. Ignoring 1 usage warnings. Please report an issue at https://github.com/r-lib/lintr/issues.
MichaelChirico commented 1 year ago

c.f. the codetools::checkUsage() output:

codetools::checkUsage({
  foo <- function() bar()
})
# <anonymous>: no visible global function definition for ‘bar’

codetools::checkUsage({
  foo <- function() {
    bar()
  }
})
# <anonymous>: no visible global function definition for ‘bar’ (:3)
AshesITR commented 1 year ago

Not really a regression, the old version just kept silent about the problem.

We should try to fix this though. At this point I'm wondering whether it might be worthwhile to fork and fix the issues in codetools directly.

MichaelChirico commented 1 year ago

would you mind having a crack at a fix?

I thought it would be as simple as making this a maybe() but I think that's having an interaction with all the other maybe()s I couldn't figure out quickly:

https://github.com/r-lib/lintr/blob/ed91a2ff3dc31631e93eb98f65bb5779606b6d59/R/object_usage_linter.R#L288

AshesITR commented 1 year ago

I can take a look, do you have MWEs for all the "Couldn't parse usage message" warnings you found?

AshesITR commented 1 year ago

It was surprisingly easy to remedy - you were almost there. Just had to make the message part fit lazily.