search

r-lib / lintr

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

Lint `deparse(substitute(x))` for `deparse1(substitute(x))` #2615

Open m-muecke opened 2 weeks ago

m-muecke commented 2 weeks ago

Since R 4.0.0 the pattern deparse1(substitute(x)) seems to be recommended over deparse(substitute(x)). See the PR for examples: https://bugs.r-project.org/show_bug.cgi?id=17671 but this most likely will have many false positives.

MichaelChirico commented 1 week ago

I think deparse(substitute(x)) is error-prone, hence why deparse1() is recommended... but I'm not sure there's no legitimate need for plain deparse(substitute(x)), hence a lot of false positives like you say. I tend to shy away from linters that have a lot of built-in false positives, especially without a really good classification of good vs. bad use cases that we can convey concisely to the user in a lint message. Do you want to flesh something like that out?

Reconstructing deparse1(), i.e. paste(deparse(.), collapse = " "), is definitely lintable, OTOH.

AshesITR commented 1 week ago

We could use it ourselves:

https://github.com/r-lib/lintr/blob/main/R/utils.R#L75