Fix #187

R/generate.R

@@ -130,7 +130,7 @@ evaluate_plan <- function(
   out[[minor]] <- NULL
   out[[major]] <- NULL
   rownames(out) <- NULL
-  return(out)
+  sanitize_plan(out)
 }
 
 evaluations <- function(
@@ -181,7 +181,7 @@ expand_plan <- function(plan, values = NULL){
   values <- rep(values, times = nrows)
   plan$target <- paste(plan$target, values, sep = "_")
   rownames(plan) <- NULL
-  return(plan)
+  sanitize_plan(plan)
 }
 
 #' @title Write commands to combine several targets into one

R/sanitize.R

@@ -7,6 +7,7 @@ sanitize_plan <- function(plan){
   if (!is.null(plan[["trigger"]])){
     assert_legal_triggers(plan[["trigger"]])
   }
+  plan$target <- repair_target_names(plan$target)
   plan[nchar(plan$target) > 0, ]
 }
 
@@ -31,7 +32,7 @@ drake_plan_columns <- function(){
 
 sanitize_targets <- function(plan, targets){
   plan <- sanitize_plan(plan)
-  targets <- str_trim(targets, side = "both")
+  targets <- repair_target_names(targets)
   sanitize_nodes(nodes = targets, choices = plan$target)
 }
 
@@ -55,3 +56,22 @@ sanitize_nodes <- function(nodes, choices){
   intersect(nodes, choices) %>%
     unique
 }
+
+repair_target_names <- function(x){
+  illegals <- c(
+    ":", "\\+", "\\-", "\\*", "\\^",
+    "\\(", "\\)", "\\[", "\\]", "^_"
+  ) %>%
+    paste(collapse = "|")
+  non_files <- x[is_not_file(x)]
+  if (any(grepl(illegals, non_files))){
+    warning("replacing illegal symbols in target names with '_'.")
+  } else {
+    return(x)
+  }
+  x <- str_trim(x, side = "both")
+  x[is_not_file(x)] <- gsub(illegals, "_", x[is_not_file(x)])
+  x <- gsub("^_", "", x)
+  x[!nchar(x)] <- "X"
+  make.unique(x, sep = "_")
+}

tests/testthat/test-workflow-plan.R

@@ -102,3 +102,49 @@ test_with_dir("check_plan() finds bad symbols", {
     command = 1)
   expect_silent(o <- check_plan(x, verbose = FALSE))
 })
+
+test_with_dir("illegal target names get fixed", {
+  pl <- data.frame(
+    target = c("_a", "a^", "a*", "a-"),
+    command = 1,
+    stringsAsFactors = FALSE
+  )
+  cache <- storr::storr_environment()
+  expect_warning(
+    con <- make(pl, cache = cache, session_info = FALSE)
+  )
+  expect_equal(
+    sort(con$plan$target),
+    sort(con$targets),
+    sort(cached(cache = cache)),
+    sort(c("a", "a_", "a__1", "a__2"))
+  )
+})
+
+test_with_dir("issue 187 on Github (from Kendon Bell)", {
+  test <- drake_plan(test = run_it(wc__))
+  out <- expect_warning(
+    evaluate_plan(test, rules = list(wc__ = list(1:4, 5:8, 9:12)))
+  )
+  out2 <- data.frame(
+    target = c("test_1_4", "test_5_8", "test_9_12"),
+    command = c("run_it(1:4)", "run_it(5:8)", "run_it(9:12)"),
+    stringsAsFactors = FALSE
+  )
+  expect_equal(out, out2)
+})
+
+test_with_dir("file names with weird characters do not get mangled", {
+  out <- data.frame(
+    target = c("'is:a:file'", "not:a:file"),
+    command = as.character(1:2),
+    stringsAsFactors = FALSE
+  )
+  out2 <- expect_warning(sanitize_plan(out))
+  out3 <- data.frame(
+    target = c("'is:a:file'", "not_a_file"),
+    command = as.character(1:2),
+    stringsAsFactors = FALSE
+  )
+  expect_equal(out2, out3)
+})