Request for a linter to detect improperly-used Suggests dependencies

[MichaelChirico]

Originally posted by @AshesITR in https://github.com/r-lib/lintr/pull/2086#pullrequestreview-1577714648

See https://cran.r-project.org/doc/manuals/r-release/R-exts.html#Suggested-packages for the CRAN rules on Suggests dependencies. We could try and detect violations statically and lint them.

[MichaelChirico]

I'm not sure how possible this is in general.... it seems it would be tough to account for branching in full generality?

It may be possible to require code inside a branch:

if (requireNamespace(Suggested, quietly = TRUE)) {
  Suggested::foo()
}

But what about outside the branch?

if (!requireNamespace(Suggested, quietly = TRUE)) {
  stop("Suggested is required")
}
Suggested::foo()

But we may be able to make progress on this for simple cases.

[MichaelChirico]

Another difficult example:

.internal_function <- function() {
  PKG::FOO()
}
force_suggests <- function(pkg) stopifnot(requireNamespace(pkg, quietly = TRUE))
exported_function <- function() {
  force_suggests("PKG")
  .internal_function()
}

i.e., we only call PKG in internal functions, after we run a helper function that ensures the Suggests package is available. It seems very difficult to avoid throwing a false positive here statically.

[AshesITR]

From code I've seen in packages, there are a few patterns that seem somewhat detectable. A sneaky way would be to check if the package in question appears as a STR_CONST in the function body before the call (or anywhere at all). This would catch many variations like using rlang::check_installed("pkg") instead of requireNamespace("pkg") or even my_custom_checker(c("pkg1", "pkg2")).

This would open the possibility of false positives only for the kind of delegated checks you show (solving that seems doable by constructing a call graph and only checking exported functions, but that will probably only ever work if we enable lintr to lint at the project level).

WDYT about this simple heuristic?