Move as and is functions closer & test together

tidyverse · Nov 29, 2023 · 0048df6 · 0048df6
1 parent 8ae7399
commit 0048df6
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 52 deletions.
diff --git a/R/table-name.R b/R/table-name.R
@@ -9,45 +9,6 @@ table_name <- function(x) {
   structure(x, class = "dbplyr_table_name")
 }
 
-as_table_name <- function(x,
-                          con,
-                          error_arg = caller_arg(x),
-                          error_call = caller_env()) {
-  check_con(con)
-
-  if (is_table_name(x)) {
-    x
-  } else if (is.sql(x)) {
-    cli::cli_warn(
-      c(
-        "{.arg {error_arg}} uses SQL where a table identifier is expected.",
-        i = "If you want to use a literal (unquoted) identifier use {.fn I} instead."
-      )
-    )
-    table_name(unclass(x))
-  } else if (inherits(x, "ident_q")) {
-    table_name(paste0(x, collapse = "."))
-  } else if (is.ident(x)) {
-    make_table_name(unclass(x), con)
-  } else if (methods::is(x, "Id")) {
-    make_table_name(x@name, con)
-  } else if (inherits(x, "dbplyr_catalog")) {
-    make_table_name(c(unclass(x$catalog), unclass(x$schema), unclass(x$table)), con)
-  } else if (inherits(x, "dbplyr_schema")) {
-    make_table_name(c(unclass(x$schema), unclass(x$table)), con)
-  } else if (inherits(x, "AsIs")) {
-    check_string(unclass(x), allow_empty = FALSE, arg = error_arg, call = error_call)
-    table_name(unclass(x))
-  } else if (is.character(x)) {
-    make_table_name(x, con, collapse = FALSE)
-  } else {
-    cli::cli_abort(
-      "{.arg {error_arg}} uses unknown specification for table name",
-      error_call = error_call
-    )
-  }
-}
-
 make_table_name <- function(x, con, collapse = TRUE) {
   needs_quote <- !vapply(x, function(x) inherits(x, "AsIs"), logical(1))
   x[needs_quote] <- sql_escape_ident(con, x[needs_quote])
@@ -138,6 +99,44 @@ is_table_id <- function(x) {
     is.character(x)
 }
 
+as_table_name <- function(x,
+                          con,
+                          error_arg = caller_arg(x),
+                          error_call = caller_env()) {
+  check_con(con)
+
+  if (is_table_name(x)) {
+    x
+  } else if (is.sql(x)) {
+    cli::cli_warn(
+      c(
+        "{.arg {error_arg}} uses SQL where a table identifier is expected.",
+        i = "If you want to use a literal (unquoted) identifier use {.fn I} instead."
+      )
+    )
+    table_name(unclass(x))
+  } else if (inherits(x, "ident_q")) {
+    table_name(paste0(x, collapse = "."))
+  } else if (is.ident(x)) {
+    make_table_name(unclass(x), con)
+  } else if (methods::is(x, "Id")) {
+    make_table_name(x@name, con)
+  } else if (inherits(x, "dbplyr_catalog")) {
+    make_table_name(c(unclass(x$catalog), unclass(x$schema), unclass(x$table)), con)
+  } else if (inherits(x, "dbplyr_schema")) {
+    make_table_name(c(unclass(x$schema), unclass(x$table)), con)
+  } else if (inherits(x, "AsIs")) {
+    check_string(unclass(x), allow_empty = FALSE, arg = error_arg, call = error_call)
+    table_name(unclass(x))
+  } else if (is.character(x)) {
+    make_table_name(x, con, collapse = FALSE)
+  } else {
+    cli::cli_abort(
+      "{.arg {error_arg}} uses unknown specification for table name",
+      error_call = error_call
+    )
+  }
+}
 
 # table source ------------------------------------------------------------
 

diff --git a/tests/testthat/_snaps/table-name.md b/tests/testthat/_snaps/table-name.md
@@ -10,6 +10,11 @@
     Condition
       Error in `as_table_name()`:
       ! `1` uses unknown specification for table name
+    Code
+      as_table_name(I(1), con)
+    Condition
+      Error:
+      ! `I(1)` must be a single string, not the number 1.
 
 # as_table_name warns when using sql
 

diff --git a/tests/testthat/test-table-name.R b/tests/testthat/test-table-name.R
@@ -4,32 +4,52 @@
 test_that("can coerce all user facing inputs", {
   con <- simulate_dbi()
 
-  expect_equal(table_name("`x`"), table_name("`x`"))
-  expect_equal(as_table_name("x", con), table_name("`x`"))
-  expect_equal(as_table_name(I("x"), con), table_name("x"))
-  expect_equal(as_table_name(ident("x"), con), table_name("`x`"))
-  expect_equal(as_table_name(ident_q("x"), con), table_name("x"))
+  x_esc <- table_name("`x`")
+  x_raw <- table_name("x")
+
+  id <- table_name("x")
+  expect_true(is_table_id(id))
+  expect_equal(id, x_raw)
+
+  id <- "x"
+  expect_true(is_table_id(id))
+  expect_equal(as_table_name(id, con), x_esc)
+
+  id <- I("x")
+  expect_true(is_table_id(id))
+  expect_equal(as_table_name(id, con), x_raw)
+
+  id <- ident("x")
+  expect_true(is_table_id(id))
+  expect_equal(as_table_name(id, con), x_esc)
+
+  id <- ident_q("x")
+  expect_true(is_table_id(id))
+  expect_equal(as_table_name(id, con), x_raw)
 
   id <- DBI::Id(schema = "foo", table = "bar")
+  expect_true(is_table_id(id))
   expect_equal(as_table_name(id, con), table_name("`foo`.`bar`"))
+
+  # strip names, simulating DBI 1.2.0
   names(id@name) <- NULL
   expect_equal(as_table_name(id, con), table_name("`foo`.`bar`"))
 
-  expect_equal(
-    as_table_name(in_schema("foo", "bar"), con),
-    table_name("`foo`.`bar`")
-  )
-  expect_equal(
-    as_table_name(in_catalog("foo", "bar", "baz"), con),
-    table_name("`foo`.`bar`.`baz`")
-  )
+  id <- in_schema("foo", "bar")
+  expect_true(is_table_id(id))
+  expect_equal(as_table_name(id, con), table_name("`foo`.`bar`"))
+
+  id <- in_catalog("foo", "bar", "baz")
+  expect_true(is_table_id(id))
+  expect_equal(as_table_name(id, con), table_name("`foo`.`bar`.`baz`"))
 })
 
 test_that("as_table_name validates its inputs", {
   con <- simulate_dbi()
   expect_snapshot(error = TRUE, {
     as_table_name("x")
     as_table_name(1, con)
+    as_table_name(I(1), con)
   })
 })