Better checking of list arguments to bind_rows()

NEWS.md

@@ -16,6 +16,8 @@
 
 * Semi- and anti-joins now preserve the order of left-hand-side data frame (#3089).
 
+* Improved error message for invalid list arguments to `bind_rows()` (#3068).
+
 * Grouping by character vectors is now faster (#2204).
 
 # dplyr 0.7.2

src/bind.cpp

@@ -79,34 +79,28 @@ void inner_vector_check(SEXP x, int nrows, int arg) {
   }
 }
 
+static
+bool is_non_data_frame_object(SEXP x) {
+  if (TYPEOF(x) != VECSXP) return false;
+  if (!OBJECT(x)) return false;
+  return !Rf_inherits(x, "data.frame");
+}
+
 static
 void rbind_vector_check(SEXP x, R_xlen_t nrows, int arg) {
+  if (!is_vector(x) || is_non_data_frame_object(x)) {
+    bad_pos_arg(arg + 1, "must be a data frame or a named atomic vector, not a {type}",
+                _["type"] = get_single_class(x));
+  }
+
   if (rows_length(x, true) != nrows) {
     bad_pos_arg(arg + 1, "must be length {expected_size}, not {actual_size}",
                 _["expected_size"] = rows_length(x, true), _["actual_size"] = nrows);
   }
 
-  switch (TYPEOF(x)) {
-  case LGLSXP:
-  case INTSXP:
-  case REALSXP:
-  case CPLXSXP:
-  case STRSXP:
-  case RAWSXP: {
-    if (vec_names(x) != R_NilValue)
-      return;
+  if (vec_names(x) == R_NilValue) {
     bad_pos_arg(arg + 1, "must have names");
   }
-  case VECSXP: {
-    if (!OBJECT(x) || Rf_inherits(x, "data.frame"))
-      return;
-    break;
-  }
-  default:
-    break;
-  }
-  bad_pos_arg(arg + 1, "must be a data frame or a named atomic vector, not a {type}",
-              _["type"] = get_single_class(x));
 }
 static
 void cbind_vector_check(SEXP x, R_xlen_t nrows, SEXP contr, int arg) {
@@ -206,7 +200,6 @@ List rbind__impl(List dots, const SymbolString& id) {
   SymbolVector names;
 
   k = 0;
-  Function enc2native("enc2native");
   for (int i = 0; i < ndata; i++) {
     Rcpp::checkUserInterrupt();
 

tests/testthat/test-binds.R

@@ -115,11 +115,17 @@ test_that("bind_rows ignores NULL", {
   expect_equal(bind_rows(list(df, NULL)), df)
 })
 
-test_that("bind_rows only accepts data frames or vectors", {
+test_that("bind_rows only accepts data frames or named vectors", {
   ll <- list(1:5, rlang::get_env())
   expect_error(
     bind_rows(ll),
-    "Argument 2 is a list, must contain atomic vectors",
+    "Argument 1 must have names",
+    fixed = TRUE
+  )
+  ll <- list(tibble(a = 1:5), rlang::get_env())
+  expect_error(
+    bind_rows(ll),
+    "Argument 2 must be a data frame or a named atomic vector, not a environment",
     fixed = TRUE
   )
 })