refactor: changed the way check_name_consistency detect package name

Author: ColinFay

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: golem
 Title: A Framework for Robust Shiny Applications
-Version: 0.5.1.9007
+Version: 0.5.1.9008
 Authors@R: c(
     person("Colin", "Fay", , "[email protected]", role = c("cre", "aut"),
            comment = c(ORCID = "0000-0001-7343-1846")),

NEWS.md

@@ -25,6 +25,8 @@
 
 - Full refactoring of the `add_*_files` and `use_*_files` functions that now all share the same behavior
 
+- The internal `check_name_consistency()` now parses the code of `app_config.R` and get the `package` arg of `system.file`, instead of doing a text based search. This allows the function to detect several calls to `system.file` and fixes the bug from #1179
+
 ## Doc
 
 - Vignettes have been renamed

R/reload.R

@@ -34,45 +34,76 @@ detach_all_attached <- function() {
 check_name_consistency <- function(pkg) {
   old_dir <- setwd(pkg)
 
-  package_name <- desc_get(keys = "Package")
+  package_name_from_desc <- desc_get(keys = "Package")
+  package_name_from_config <- c()
+
   pth <- fs_path(
     pkg,
     "R",
     "app_config.R"
   )
-  app_config <- readLines(pth)
+  app_config <- parse(file = pth)
 
-  where_system.file <- app_config[
-    grep(
-      "system.file",
-      app_config
-    )
-  ]
+  lapply(
+    app_config,
+    function(expr) {
+      codetools::walkCode(
+        expr,
+        codetools::makeCodeWalker(
+          handler = function(e, w) return(NULL),
+          call = function(expr, w) {
+            fn <- expr[[1]]
+            if (
+              is.symbol(fn) &&
+                as.character(fn) == "system.file"
+            ) {
+              args <- as.list(expr)[-1]
+              package_name_from_config <<- c(
+                package_name_from_config,
+                args$package
+              )
+            }
+            for (e in as.list(expr)[-1L]) {
+              codetools::walkCode(e, w)
+            }
+          },
+          leaf = function(e, w) return(NULL)
+        )
+      )
+    }
+  )
 
   setwd(old_dir)
 
-  if (grepl(
-    package_name,
-    where_system.file
-  )) {
+  if (
+    length(
+      unique(
+        c(
+          package_name_from_config,
+          package_name_from_desc
+        )
+      )
+    ) ==
+      1
+  ) {
     return(invisible(TRUE))
   } else {
     stop(
       call. = FALSE,
       "Package name does not match in DESCRIPTION and `app_sys()`.\n",
       "\n",
       sprintf(
-        "DESCRIPTION: '%s'\n",
-        package_name
+        "In DESCRIPTION: '%s'\n",
+        package_name_from_desc
       ),
       sprintf(
-        "R/app_config.R - app_sys(): '%s'\n",
-        where_system.file
+        "R/app_config.R : '%s'\n",
+        paste(package_name_from_config, collapse = ", ")
       ),
       "\n",
       sprintf(
         "Please make both these names match before continuing, for example using golem::set_golem_name('%s')",
-        package_name
+        package_name_from_desc
       )
     )
   }

tests/testthat/test-reload.R

@@ -24,6 +24,16 @@ test_that(
       )
       expect_error(check_name_consistency("."))
     })
+      run_quietly_in_a_dummy_golem({
+        res <- check_name_consistency(".")
+        expect_true(res)
+        write(
+          "app_sys_2 <- function(...){system.file(..., package = 'blabla')}",
+          "R/app_config.R",
+          append = TRUE
+        )
+        expect_error(check_name_consistency("."))
+      })
   }
 )