[LINT_BUG]: missing environment variable creates error in box_unused_attached_mod_linter

[caldwellst]

box.linters version

0.9.1

Sample source code to lint

I am unable to create a simple reprex of this issue. However you can see the failure in this run. The eventual source of the error (connected through multiple box::use() calls, is this function here:

#' Gets an environment variable value, returning an error if it is not set
#'
#' @param env_var_name Name of environment variable
#'
#' @returns Value of environment variable, unless it is not set
#' @export
get_env <- function(env_var_name, output = TRUE) {
  val <- Sys.getenv(env_var_name)
  if (!nzchar(val)) {
    error_message <- sprintf("Environment variable '%s' is empty or not set.", env_var_name)
    log_error(error_message)
    stop(call. = FALSE)
  } else if (output) {
    val
  }
}

Lint command used

lintr::lint(
  code,
  linters = lintr::linters_with_defaults(defaults = box.linters::box_default_linters)
)

Lint result

The lint fails when traversing functions that check the existence of environment variables. The first script, the one the error is generated at, is here: https://github.com/OCHA-DAP/hdx-signals/blob/feature/box-lint/src-static/update_acled_info.R

This script imports a module cs = src/utils/cloud_storage. https://github.com/OCHA-DAP/hdx-signals/blob/736e8c71c9bd31d441308b11ee1d64f533f15870/src/utils/cloud_storage.R#L208

Inside that module, it uses get_env$get_env(), which returns an env var but throws an error if it doesn't exist. https://github.com/OCHA-DAP/hdx-signals/blob/feature/box-lint/src/utils/get_env.R.

This is the error we receive in the lint run. I can remove environment variables from my machine and recreate the error locally, so it is not solely present on remote runs. If I delete the file that it is first detected in, any file that imports cs = src/utils/cloud_storage (and thus uses get_env()) also throws up the error if the env variables are not present.

Expected result

When using the default linters in {lintr}, we were receiving no such errors.

I have tried to create various minimal file directories and files to test linting and create an issue on the box.linters repo. However, I cannot reproduce the error with any simple code examples. I have tried multiple nested imports, the use of switch(), hardcoding or not the env variable check. None of these recreate the error, which leads me to believe there is something particularly unique going in this complex series of calls that is generating the error.

Happy to help debug this and apologies I couldn't create this issue with a better reprex. Hoping you might already have an inkling of what is driving this, or a potential set of things to try and explore.

[radbasa]

At first inspection, this might be related to #110

box::use(../src/utils/location_codes)
box::use(cs = ../src/utils/cloud_storage)
box::use(../src/utils/hs_logger)

https://github.com/OCHA-DAP/hdx-signals/blob/89a4012133a295e8e40ad5fd6d894c9f07fb0593/src-static/update_acled_info.R#L9-L11

We'll take a look.

[caldwellst]

It might be, but this is occurring only in the feature/box-lint branch where I've shifted the entire repo based on your recommendations in other issues. The branch sets the lintr base path to the working directory and uses only two box::use() statements, one for packages and one for modules. So the file you linked (and all others) no longer has any relevant pathing https://github.com/OCHA-DAP/hdx-signals/blob/feature/box-lint/src-static/update_acled_info.R#L9-L11. The entire library on that branch passes box.linters::box_default_linters now and only fails in the case that environment variables are missing.

[radbasa]

@caldwellst , we hope you don't mind. We cloned your repo and tested with the feature/box-lint branch. We have reproduced the error about the environment variable. We have tracked the error down to one of the functions we use from the {box} package to get the exported functions and objects of a box module. We will investigate further. We will also try to build a simpler reprex for this.

Thanks for your valuable input!

[caldwellst]

Absolutely don't mind and thanks for the support. Really excited to get using your package as it's clearly a missing piece of the {box} workflow currently.