Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

undesirable_function_linter() should look for function calls only #556

Closed
jennybc opened this issue Nov 2, 2020 · 0 comments · Fixed by #557
Closed

undesirable_function_linter() should look for function calls only #556

jennybc opened this issue Nov 2, 2020 · 0 comments · Fixed by #557

Comments

@jennybc
Copy link
Member

jennybc commented Nov 2, 2020

I think the undesirable_function_linter() should only look for function calls. The context is that I want to use lintr to keep watch for base file system functions creeping into usethis (r-lib/usethis#531, r-lib/fs#85). I can almost do what I want with the undesirable_function_linter().

But it reports tons of false positives, i.e. warns when it should not.

dir() is one of the functions I'm linting for. But throughout the package, below both R/ and tests/, the name dir is frequently used for the path to a directory. So the linter reports dozens of false warnings.

Here's a small reprex with current lintr vs. after the change in my PR (#557):

library(lintr)

lint_this <- function(this, ...) {
  file <- tempfile()
  on.exit(unlink(file))
  writeLines(this, con = file, sep = "\n")
  lint(file, ...)
}

linter <- undesirable_function_linter(fun = c("dir" = NA))

# warning is correct: I'm calling dir()
lint_this('dir(".")', linter)
#> /private/var/folders/pv/l2mbh7j90y5_vslh_1wyqxj00000gn/T/RtmpQbfI18/file121123553fe9e:1:1: warning: Function "dir" is undesirable.
#> dir(".")
#> ^~~

# warning is WRONG: I'm defining an object named 'dir'
lint_this("dir <- 1:3", linter)
#> /private/var/folders/pv/l2mbh7j90y5_vslh_1wyqxj00000gn/T/RtmpQbfI18/file121121d71526c:1:1: warning: Function "dir" is undesirable.
#> dir <- 1:3
#> ^~~

devtools::load_all("~/rrr/lintr")
#> ℹ Loading lintr

linter <- undesirable_function_linter(fun = c("dir" = NA))

# warning is correct: I'm calling dir()
lint_this('dir(".")', linter)
#> /private/var/folders/pv/l2mbh7j90y5_vslh_1wyqxj00000gn/T/RtmpQbfI18/file121127ada3f8d:1:1: warning: Function "dir" is undesirable.
#> dir(".")
#> ^~~

# lack of warning is correct: I'm defining an object named 'dir'
lint_this("dir <- 1:3", linter)

Created on 2020-11-01 by the reprex package (v0.3.0.9001)

MichaelChirico added a commit that referenced this issue Nov 30, 2020
* Option to control whether undesirable function names can appear as symbol

Closes #556

* Use inlined source

* Simplify

* Add expectation

* Remove examples

* Make test more concise

* operator spacing

Co-authored-by: AshesITR <alexander.rosenstock@web.de>
Co-authored-by: Michael Chirico <michaelchirico4@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant