From a1faf84ae5ed6c8eefd80a4fd37b2fd7a0698321 Mon Sep 17 00:00:00 2001
From: Etienne Bacher <52219252+etiennebacher@users.noreply.github.com>
Date: Mon, 22 Apr 2024 09:06:12 +0200
Subject: [PATCH] Prevent unnamed non-`Field` elements in `pl$Struct()`

---
 NEWS.md                        |  6 +++++
 R/datatype.R                   | 48 ++++++++++++++++------------------
 man/DataType_Struct.Rd         | 10 +++----
 tests/testthat/test-datatype.R |  2 +-
 4 files changed, 34 insertions(+), 32 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index 074155557..b26aa7d81 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -2,6 +2,12 @@
 
 ## polars (development version)
 
+### Breaking changes
+
+- `pl$Struct()` now only accepts named inputs and objects of class `RPolarsField`.
+  For example, `pl$Struct(pl$Boolean)` doesn't work anymore and should be named
+  like `pl$Struct(a = pl$Boolean)` (#1053).
+
 ## polars 0.16.1
 
 This is a small hot-fix release to update dependent Rust polars to 0.39.1 (#1042).
diff --git a/R/datatype.R b/R/datatype.R
index c90fe24b7..ec55a7af5 100644
--- a/R/datatype.R
+++ b/R/datatype.R
@@ -211,15 +211,15 @@ DataType_Duration = function(time_unit = "us") {
 #' multiple ways to create columns of data type `Struct` in a `DataFrame` or
 #' a `Series`, see the examples.
 #'
-#' @param ... RPolarsDataType objects
-#' @return a list DataType with an inner DataType
+#' @param ... Either named inputs of the form `field_name = datatype` or objects
+#' of class `RPolarsField` created by [`pl$Field()`][pl_Field].
+#' @return A Struct DataType containing a list of Fields
 #' @examples
 #' # create a Struct-DataType
-#' pl$Struct(pl$Boolean)
-#' pl$Struct(foo = pl$Int32, bar = pl$Float64)
+#' pl$Struct(foo = pl$Int32, pl$Field("bar", pl$Boolean))
 #'
 #' # check if an element is any kind of Struct()
-#' test = pl$Struct(pl$UInt64)
+#' test = pl$Struct(a = pl$UInt64)
 #' pl$same_outer_dt(test, pl$Struct())
 #'
 #' # `test` is a type of Struct, but it doesn't mean it is equal to an empty Struct
@@ -251,34 +251,30 @@ DataType_Duration = function(time_unit = "us") {
 #'
 #' out$schema
 DataType_Struct = function(...) {
+  uw = \(res) unwrap(res, "in pl$Struct():")
+  err_message = Err_plain("`pl$Struct()` only accepts named inputs or input of class RPolarsField.")
   result({
     largs = list2(...)
     if (length(largs) >= 1 && is.list(largs[[1]])) {
       largs = largs[[1]]
-      element_name = "list element"
-    } else {
-      element_name = "positional argument"
     }
-    mapply(
-      names(largs) %||% character(length(largs)),
-      largs,
-      seq_along(largs),
-      FUN = \(name, arg, i) {
-        if (inherits(arg, "RPolarsDataType")) {
-          return(pl$Field(name, arg))
-        }
-        if (inherits(arg, "RPolarsRField")) {
-          return(arg)
-        }
-        stop(sprintf(
-          "%s [%s] {name:'%s', value:%s} must either be a Field (pl$Field) or a named DataType",
-          element_name, i, name, arg
-        ))
-      }, SIMPLIFY = FALSE
-    )
+    lapply(seq_along(largs), function(x) {
+      name = names(largs)[x]
+      dtype = largs[[x]]
+      if (inherits(dtype, "RPolarsRField")) {
+        return(dtype)
+      }
+      if (is.null(name)) {
+        err_message |> uw()
+      }
+      if (inherits(dtype, "RPolarsDataType")) {
+        return(pl$Field(name, dtype))
+      }
+      err_message |> uw()
+    })
   }) |>
     and_then(DataType$new_struct) |>
-    unwrap("in pl$Struct:")
+    uw()
 }
 
 #' Create Array DataType
diff --git a/man/DataType_Struct.Rd b/man/DataType_Struct.Rd
index 788fbdbe0..152c3ab49 100644
--- a/man/DataType_Struct.Rd
+++ b/man/DataType_Struct.Rd
@@ -7,10 +7,11 @@
 DataType_Struct(...)
 }
 \arguments{
-\item{...}{RPolarsDataType objects}
+\item{...}{Either named inputs of the form \code{field_name = datatype} or objects
+of class \code{RPolarsField} created by \code{\link[=pl_Field]{pl$Field()}}.}
 }
 \value{
-a list DataType with an inner DataType
+A Struct DataType containing a list of Fields
 }
 \description{
 One can create a \code{Struct} data type with \code{pl$Struct()}. There are also
@@ -19,11 +20,10 @@ a \code{Series}, see the examples.
 }
 \examples{
 # create a Struct-DataType
-pl$Struct(pl$Boolean)
-pl$Struct(foo = pl$Int32, bar = pl$Float64)
+pl$Struct(foo = pl$Int32, pl$Field("bar", pl$Boolean))
 
 # check if an element is any kind of Struct()
-test = pl$Struct(pl$UInt64)
+test = pl$Struct(a = pl$UInt64)
 pl$same_outer_dt(test, pl$Struct())
 
 # `test` is a type of Struct, but it doesn't mean it is equal to an empty Struct
diff --git a/tests/testthat/test-datatype.R b/tests/testthat/test-datatype.R
index 7cd8f879e..e19016cd3 100644
--- a/tests/testthat/test-datatype.R
+++ b/tests/testthat/test-datatype.R
@@ -27,7 +27,7 @@ test_that("plStruct", {
   # wrong uses
   expect_grepl_error(
     pl$Struct(bin = pl$Binary, pl$Boolean, "abc"),
-    "must either be a Field"
+    "only accepts named inputs or input of class RPolarsField."
   )
 })