Potentially dangerous functions

[gaborcsardi]

These come to mind, with various levels of danger:

• attach/detach
• setwd
• sapply
• library/require

attach/detach is definitely bad, but I am not quite sure about the others. library/require is only bad in a package, and R CMD check already warns for it. setwd is also only bad in a package imo, and if not used together with on.exit.

Would you like some of these in lintr?

[jimhester]

Yeah I had on open issue with most of those #48, but I clearly haven't worked on them. If you think they would be useful PR welcome!

[rkrug]

I can see why attach/detach, setwd and library / require are bad - but why sapply? Am I missing
something here? I use sapply all the time.

[gaborcsardi]

OK, here is the thing. I realized that just looking for (say) attach function calls might not be good enough, and we also have to make sure that the package does not define attach as a function or a local variable, although local variables are easy from the parse data.

So the parse data is not enough to implement these reliably.

Maybe we can say, that not too many people use these function names, so we won't have many false positives, but how about T and F (from #48)? These variable names are probably more common. Btw. even tools::checkTnF() gets this wrong AFAICT.

My point is that this is a generic problem, but I am not sure if you want to solve it in lintr.

[gaborcsardi]

@rkrug sapply is not type safe, it returns a different data type depending on the input, and on the input length. In particular

sapply(1:10, paste)

returns a character vector, and

sapply(integer(), paste)

returns a list. This often leads to errors in edge cases. A better alternative is vapply.

[rkrug]

@gaborcsardi Thanks - makes sense. And it explains some errors I got in my analysis. Never looked at vapply - seems to be a very good alternative.

[russHyde]

undesirable_function_linter and undesirable_operator_linter were added since this issue was opened. Suggest closing the issue