Add missing_package_linter

[renkun-ken]

Closes #536

• Check library calls (library, require, loadNamespace, requireNamespace)
• Test cases

[renkun-ken]

@jimhester I'm wondering which makes more sense: check both library() calls and namepsace:: calls in one linter or in separate linters so that users could choose either or both accordingly?

[AshesITR]

I'd suggest to separate library / require / loadNamespace (which will error if a package is not installed)
from requireNamespace, :: and :::.

The latter three are often used in packages to conditionally use functionality.
The following shouldn't lint with the library linter imho.

if (requireNamespace("data.table", quietly = TRUE)) {
  # do data.table stuff, e.g.
  .data <- data.table::as.data.table(.data)
} else {
  # do data.frame stuff
  .data <- as.data.frame(.data)
}

[renkun-ken]

@AshesITR I agree. So I won't check namespace:: calls then. I'll add some test cases for this.

BTW, do you think the linting message of missing packages should be an error or warning? Do we have a clear definition about what should be error and warning?

[AshesITR]

Currently the only error linter is the parse errors.
I'd say uninstalled packages should be treated like the object_usage_linter which looks for missing bindings. That would be warning.

[renkun-ken]

warning makes good sense. Updated.

[renkun-ken]

Any idea why test jobs against old releases failed?

https://travis-ci.org/github/jimhester/lintr/jobs/740571821

[AshesITR]

sprintf("%s", as.name("asd")) seems to fail in oldrels.
I think adding as.character() to all formatted symbols might fix it.

[AshesITR]

Just looked into R 4.0.3s base::format.default().

The implementation for mode(x) == "name" is

deparse(x, backtick = FALSE)

This is in R >= 4.0.0

[AshesITR]

A minimal compatibility fix would be adding this code:

if (getRversion() < '4.0.0') {
  # format.default handles names since R 4.0.0
  format.name <- function(x) deparse(x, backtick = FALSE)
}

But I'd prefer to use as.character() at the call sites if that works, since this way we could avoid R version specific patch code.

[AshesITR]

the merge unfortunately necessitates another document() :(

[MichaelChirico]

could this lib.paths be an argument to this linter? or perhaps known_packages as an argument. would make it more flexible to check code statically in scenarios where the packages may not be installed or the linter is run with a funny PATH

[renkun-ken]

#551 (comment)