r-lib / lintr

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

False positive in `undesirable_function_linter()` #2584

Closed IndrajeetPatil closed 1 month ago

IndrajeetPatil commented 1 month ago

message is a parameter name, but it's mistaken to be the function name:

library(lintr)

f <- function(message) print(message)
f("hi")
#> [1] "hi"

lint(
  text = "f <- function(message) print(message)", 
  linters = undesirable_function_linter(modify_defaults(
    defaults = default_undesirable_functions,
    message = "use cli::cli_inform()"
  ))
)
#> <text>:1:30: style: [undesirable_function_linter] Avoid undesirable function "message". As an alternative, use cli::cli_inform().
#> f <- function(message) print(message)
#>                              ^~~~~~~

Created on 2024-05-25 with reprex v2.1.0

IndrajeetPatil commented 1 month ago

IINM, we should be able to distinguish between these two cases by checking if it's SYMBOL_FUNCTION_CALL versus SYMBOL_FORMALS.

MichaelChirico commented 1 month ago

but that's not SYMBOL_FORMALS, it's SYMBOL. you can ignore it with symbol_is_undesirable=FALSE.

MichaelChirico commented 1 month ago

note that in the same function you could do sapply(strs, message) and it would be the "undesirable" function applied, not the function argument. so going down the route of "try and look in parent for matching SYMBOL_FORMALS" is very error prone for many reasons.

IndrajeetPatil commented 1 month ago

You are right, forgot about this argument!

library(lintr)

lint(
  text = "f <- function(message) print(message)", 
  linters = undesirable_function_linter(modify_defaults(
    defaults = default_undesirable_functions,
    message = "use cli::cli_inform()"
  ), symbol_is_undesirable = FALSE)
)

Created on 2024-05-25 with reprex v2.1.0