Improve handling of invalid _pkgdown.yaml (#2118)

* Error, regardless of whether we're on CI or not (Fixes #2055)
* New `check_pkgdown()` for lightweight check of validity (Fixes #2056).

NAMESPACE

@@ -123,6 +123,7 @@ export(build_search)
 export(build_site)
 export(build_site_github_pages)
 export(build_tutorials)
+export(check_pkgdown)
 export(clean_site)
 export(data_template)
 export(deploy_site_github)

NEWS.md

@@ -1,5 +1,11 @@
 # pkgdown (development version)
 
+* Invalid `_pkgdown.yml` now consistently generates errors both locally and on
+  CI (#2055).
+
+* New `check_pkgdown()` provides a lightweight way to check that your 
+  `_pkgdown.yml` is valid without building the site (#2056).
+
 * If you've included the logo in the a topic description (as on the 
   package reference page), it's automatically stripped (#2083).
 

R/build-articles.R

@@ -364,11 +364,9 @@ data_articles_index <- function(pkg = ".") {
   missing <- !(pkg$vignettes$name %in% listed)
 
   if (any(missing)) {
-    warning(
+    abort(
       "Vignettes missing from index: ",
-      paste(pkg$vignettes$name[missing], collapse = ", "),
-      call. =  FALSE,
-      immediate. = TRUE
+      paste(pkg$vignettes$name[missing], collapse = ", ")
     )
   }
 
@@ -380,12 +378,7 @@ data_articles_index <- function(pkg = ".") {
 
 data_articles_index_section <- function(section, pkg) {
   if (!set_contains(names(section), c("title", "contents"))) {
-    warning(
-      "Section must have components `title`, `contents`",
-      call. = FALSE,
-      immediate. = TRUE
-    )
-    return(NULL)
+    abort("Section must have components `title`, `contents`")
   }
 
   # Match topics against any aliases
@@ -420,6 +413,11 @@ select_vignettes <- function(match_strings, vignettes) {
 default_articles_index <- function(pkg = ".") {
   pkg <- as_pkgdown(pkg)
 
+  if (nrow(pkg$vignettes) == 0L) {
+    return(NULL)
+  }
+
+
   print_yaml(list(
     list(
       title = tr_("All vignettes"),

R/build-favicons.R

@@ -93,10 +93,10 @@ build_favicons <- function(pkg = ".", overwrite = FALSE) {
     utils::unzip(tmp, exdir = path(pkg$src_path, "pkgdown", "favicon"))
   },
   warning = function(e) {
-    stop("Your logo file couldn't be processed and may be corrupt.", call. = FALSE)
+    abort("Your logo file couldn't be processed and may be corrupt.", parent = e)
   },
   error = function(e) {
-    stop("Your logo file couldn't be processed and may be corrupt.", call. = FALSE)
+    abort("Your logo file couldn't be processed and may be corrupt.", parent = e)
   })
 
   invisible()

R/build-reference-index.R

@@ -141,16 +141,11 @@ check_missing_topics <- function(rows, pkg) {
   in_index <- pkg$topics$name %in% all_topics
 
   missing <- !in_index & !pkg$topics$internal
+
   if (any(missing)) {
-    text <- sprintf("Topics missing from index: %s", unname(pkg$topics$name[missing]))
-    if (on_ci()) {
-      abort(text)
-    } else {
-      warn(text)
-    }
+    abort(c(
+      "All topics must be included in reference index",
+      paste0("Missing topics: ", paste0(pkg$topics$name[missing], collapse = ", "))
+    ))
   }
 }
-
-on_ci <- function() {
-  isTRUE(as.logical(Sys.getenv("CI")))
-}

R/check.R

@@ -0,0 +1,17 @@
+#' Check `_pkgdown.yml`
+#'
+#' Check that your `_pkgdown.yml` is valid without building the whole
+#' site.
+#'
+#' @export
+#' @inheritParams as_pkgdown
+check_pkgdown <- function(pkg = ".") {
+  pkg <- as_pkgdown(pkg)
+
+  data_open_graph(pkg)
+  data_articles_index(pkg)
+  data_reference_index(pkg)
+
+  inform("No problems found")
+}
+

R/topics.R

@@ -10,7 +10,7 @@ select_topics <- function(match_strings, topics, check = FALSE) {
   # If none of the specified topics have a match, return no topics
   if (purrr::every(indexes, is_empty)) {
     if (check) {
-      warn("No topics matched in '_pkgdown.yml'. No topics selected.")
+      abort("No topics matched in '_pkgdown.yml'. No topics selected.")
     }
     return(integer())
   }
@@ -165,7 +165,7 @@ match_eval <- function(string, env) {
 }
 
 topic_must <- function(message, topic) {
-  warn(c(
+  abort(c(
     paste0("In '_pkgdown.yml', topic must ", message),
     x = paste0("Not ", encodeString(topic, quote = "'"))
   ))

pkgdown/_pkgdown.yml

@@ -64,6 +64,7 @@ reference:
   contents:
   - starts_with("deploy_")
   - build_site_github_pages
+  - check_pkgdown
 
 - title: Templates
   contents:

tests/testthat/_snaps/build-reference-index.md

@@ -45,6 +45,15 @@
       has_icons: no
       
 
+# warns if missing topics
+
+    Code
+      data_reference_index(pkg)
+    Condition
+      Error in `check_missing_topics()`:
+      ! All topics must be included in reference index
+      * Missing topics: c, e, ?
+
 # errors well when a content entry is empty
 
     Item 2 in section 1 in reference in '_pkgdown.yml' is empty.

tests/testthat/_snaps/check.md

@@ -0,0 +1,24 @@
+# fails if reference index incomplete
+
+    Code
+      check_pkgdown(pkg)
+    Condition
+      Error in `check_missing_topics()`:
+      ! All topics must be included in reference index
+      * Missing topics: ?
+
+# fails if article index incomplete
+
+    Code
+      check_pkgdown(pkg)
+    Condition
+      Error in `data_articles_index()`:
+      ! Vignettes missing from index: 
+
+# informs if everything is ok
+
+    Code
+      check_pkgdown(pkg)
+    Message
+      No problems found
+

tests/testthat/_snaps/topics.md

@@ -3,36 +3,54 @@
     Code
       t <- select_topics("x + ", topics)
     Condition
-      Warning:
-      In '_pkgdown.yml', topic must be valid R code
+      Error in `topic_must()`:
+      ! In '_pkgdown.yml', topic must be valid R code
       x Not 'x + '
     Code
       t <- select_topics("y", topics)
     Condition
-      Warning:
-      In '_pkgdown.yml', topic must be a known topic name or alias
+      Error in `topic_must()`:
+      ! In '_pkgdown.yml', topic must be a known topic name or alias
       x Not 'y'
     Code
       t <- select_topics("paste(1)", topics)
     Condition
-      Warning:
-      In '_pkgdown.yml', topic must be a known selector function
+      Error in `topic_must()`:
+      ! In '_pkgdown.yml', topic must be a known selector function
       x Not 'paste(1)'
     Code
       t <- select_topics("starts_with", topics)
     Condition
-      Warning:
-      In '_pkgdown.yml', topic must be a known topic name or alias
+      Error in `topic_must()`:
+      ! In '_pkgdown.yml', topic must be a known topic name or alias
       x Not 'starts_with'
     Code
       t <- select_topics("1", topics)
     Condition
-      Warning:
-      In '_pkgdown.yml', topic must be a string or function call
+      Error in `topic_must()`:
+      ! In '_pkgdown.yml', topic must be a string or function call
       x Not '1'
     Code
       t <- select_topics("starts_with('y')", topics, check = TRUE)
     Condition
-      Warning:
-      No topics matched in '_pkgdown.yml'. No topics selected.
+      Error in `select_topics()`:
+      ! No topics matched in '_pkgdown.yml'. No topics selected.
+
+# can select by name or alias
+
+    Code
+      select_topics("a4", topics)
+    Condition
+      Error in `topic_must()`:
+      ! In '_pkgdown.yml', topic must be a known topic name or alias
+      x Not 'a4'
+
+# an unmatched selection generates a warning
+
+    Code
+      select_topics(c("a", "starts_with('unmatched')"), topics, check = TRUE)
+    Condition
+      Error in `topic_must()`:
+      ! In '_pkgdown.yml', topic must match a function or concept
+      x Not 'starts_with(\'unmatched\')'
 

tests/testthat/test-build-reference-index.R

@@ -57,11 +57,7 @@ test_that("warns if missing topics", {
   meta <- list(reference = ref)
   pkg <- as_pkgdown(test_path("assets/reference"), override = meta)
 
-  withr::local_envvar(c(CI = "false"))
-  expect_warning(data_reference_index(pkg), "Topics missing")
-
-  withr::local_envvar(c(CI = "true"))
-  expect_error(data_reference_index(pkg), "Topics missing")
+  expect_snapshot(data_reference_index(pkg), error = TRUE)
 })
 
 test_that("default reference includes all functions", {

tests/testthat/test-check.R

@@ -0,0 +1,23 @@
+test_that("fails if reference index incomplete", {
+  pkg <- local_pkgdown_site(test_path("assets/reference"), meta = "
+    reference:
+     - title: Title
+       contents: [a, b, c, e]
+  ")
+  expect_snapshot(check_pkgdown(pkg), error = TRUE)
+})
+
+
+test_that("fails if article index incomplete", {
+  pkg <- local_pkgdown_site(test_path("assets/articles"), meta = "
+    articles:
+     - title: Title
+       contents: [starts_with('html'), standard, toc-false, widget]
+  ")
+  expect_snapshot(check_pkgdown(pkg), error = TRUE)
+})
+
+test_that("informs if everything is ok", {
+  pkg <- local_pkgdown_site(test_path("assets/reference"))
+  expect_snapshot(check_pkgdown(pkg))
+})

tests/testthat/test-topics.R

@@ -5,7 +5,7 @@ test_that("bad inputs give informative warnings", {
     "x",   c("x", "x1"), FALSE,      character(),
   )
 
-  expect_snapshot({
+  expect_snapshot(error = TRUE, {
     t <- select_topics("x + ", topics)
     t <- select_topics("y", topics)
     t <- select_topics("paste(1)", topics)
@@ -34,7 +34,7 @@ test_that("can select by name or alias", {
   expect_equal(select_topics("b-a", topics), 3)
 
   # Or missing
-  expect_warning(select_topics("a4", topics), "known topic")
+  expect_snapshot(select_topics("a4", topics), error = TRUE)
 })
 
 test_that("selection preserves original order", {
@@ -120,8 +120,7 @@ test_that("an unmatched selection generates a warning", {
     "d",   "d",           TRUE,
   )
 
-  expect_warning(
+  expect_snapshot(error = TRUE,
     select_topics(c("a", "starts_with('unmatched')"), topics, check = TRUE),
-    "topic must match a function or concept"
   )
 })