code linter for timezone argument

[jonathancallahan]

I recently got questions from a user of the in-development AirSensor package about timezone shifting. In reviewing our code I realized, to my horror, that the code is inconsistent in passing the timezone argument to functions that accept it.

Code developed at Mazama Science needs to always, always, ALWAYS specify timezones! Given that I invariably hire inexperienced R programmers, the burden for this guarantee falls on me. So let's automate this. I reviewed the lintr package and decided that it is too heavyweight.

The top priority is to have something quickly that I can at least use by hand to test code. This will include:

• findFunctionCalls()
• test_usesArgument() for a single source file
• enhanced test_usesArgument() where we can specify a directory and a set of file extensions and (recursive = TRUE?) and have it test every file

I came up with the following barely working example for what I want:

test_usesArgument_B <- function(
  packageFUNC = NULL,
  functionName = NULL,
  argumentName = NULL
) {

  argumentPattern <- paste0(argumentName,"=|",argumentName,"\\s=")
  
  functionString <- paste0( as.character(rlang::fn_body(packageFUNC)), collapse="\n")
  
  testResult <- stringr::str_detect(findFunctionCalls(functionString, functionName), argumentPattern)
  
  # Could optionally spit out offending calls
  
  return(testResult)
  
}

test_usesArgument <- function(
  file = NULL,
  functionName = NULL,
  argumentName = NULL
) {
  
  argumentPattern <- paste0(argumentName,"=|",argumentName,"\\s=")
  
  functionString <- readr::read_file(file)
  
  testResult <- stringr::str_detect(findFunctionCalls(functionString, functionName), argumentPattern)
  
  # Could optionally spit out offending calls
  
  return(testResult)
  
}

findFunctionCalls <- function(
  functionString = NULL,
  functionName = NULL
) {
  
  functionOpen <- paste0(functionName, "\\(")

  openParens <- stringr::str_locate_all(functionString, "\\(")[[1]][,1]
  closeParens <- stringr::str_locate_all(functionString, "\\)")[[1]][,1]
  
  # TODO:  A better regex above would avoid matching escaped parens so that
  # TODO:  we wouldn't need the check below
  
  if ( length(openParens) != length(closeParens) )
    stop("Unmatched parens (Are you using strings with single escaped parens?)")
  
  functionStarts <- stringr::str_locate_all(functionString, functionOpen)[[1]][,1]
  functionEnds <- stringr::str_locate_all(functionString, functionOpen)[[1]][,2]
  
  indices <- which(openParens %in% (functionEnds))
  
  starts <- functionStarts
  ends <- closeParens[indices]
  
  functionCallStrings <- stringr::str_sub(functionString, starts, ends)
  
  return(functionCallStrings)
  
}

# ===== DEBUGGING ==============================================================

if ( FALSE ) {
 
  file <- "R/pas_load.R"
  functionName <- "strftime"
  
  # Test on source code in the AirSensor package
  stringr::str_detect(findFunctionCalls(readr::read_file(file), "strftime"), "tz=|tz\\s=")
  #[1] TRUE TRUE TRUE TRUE
  all(stringr::str_detect(findFunctionCalls(readr::read_file(file), "strftime"), "tz=|tz\\s="))
  #[1] TRUE  
  
  test_usesArgument("R/pas_load.R", "strftime", "tz")
  #[1] TRUE TRUE TRUE TRUE
  
  test_usesArgument_B(PWFSLSmoke::monitor_dailyBarplot, "strftime", "tz")
  #[1] TRUE TRUE
  
}

Ideally, we could include a test inside of a package that looked something like this:

context("test-timezone-usage")

test_that("timezone is always specified", {
  expect_true( all( MazamaCoreUtils::test_usesArgument(<package source>, "strftime", "timezone") )
})
  expect_true( all( MazamaCoreUtils::test_usesArgument(<package source>, "strptime", "timezone") )
})
... ditto for "lubridate::ymd", etc. etc. ...

Here is what I am already doing with the prototype functionality:

  files <- list.files("R", full.names = TRUE)
  for ( file in files ) {
    
    result <- try({
      test_usesArgument(readr::read_file(file), "strftime", "tz")
    }, silent = TRUE)
    if ( "try-error" %in% class(result) ) {
      cat(file)
      cat("\n")
      cat(geterrmessage())
      cat("\n\n")
    } else if ( length(result) > 0 && !any(result) ) {
      cat(file)
      cat("\n")
      cat(result)
      cat("\n\n")
    }
    
  }