Ensure that vec_ptype_common() always finalizes its output (#2102)

* Ensure that `vec_ptype_common()` always finalizes its output

Even user supplied `ptype`s

* Typo

Author: DavisVaughan

src/ptype-common.c

@@ -54,6 +54,8 @@ r_obj* ffi_ptype_common_params(r_obj* ffi_call, r_obj* op, r_obj* args, r_obj* e
   return out;
 }
 
+// Invariant of `vec_ptype_common()` is that the output is always a finalised `ptype`,
+// even if the user provided their own
 r_obj* vec_ptype_common(
   r_obj* dots,
   r_obj* ptype,
@@ -62,7 +64,7 @@ r_obj* vec_ptype_common(
   struct r_lazy call
 ) {
   if (ptype != r_null) {
-    return vec_ptype(ptype, vec_args.dot_ptype, call);
+    return vec_ptype_final(ptype, vec_args.dot_ptype, call);
   }
 
   if (r_is_true(r_peek_option("vctrs.no_guessing"))) {

tests/testthat/_snaps/cast.md

@@ -83,3 +83,11 @@
       i Please use `allow_lossy_cast()` instead.
       i We detected a lossy transformation from `x` <fct> to `to` <fct>. The result will contain lower-resolution values or missing values. To suppress this warning, wrap your code with `allow_lossy_cast()`.
 
+# can cast to unspecified `NA` with `vec_cast()` and `vec_cast_common()` (#2099)
+
+    Code
+      vec_cast(TRUE, to = unspecified(1))
+    Condition
+      Error:
+      ! Can't convert `TRUE` <logical> to <vctrs_unspecified>.
+

tests/testthat/test-cast.R

@@ -337,3 +337,23 @@ test_that("bare-type fallback for df-cast works", {
 
   expect_error(vec_rbind(gdf, gdf), NA)
 })
+
+test_that("can cast to unspecified `NA` with `vec_cast()` and `vec_cast_common()` (#2099)", {
+  # In the `vec_cast()` case no `vec_ptype()` call is made, which means that no
+  # finalization step is required. In the `vec_cast_common()` case, the
+  # underlying call to `vec_ptype_common()` calls `vec_ptype(NA)` but also
+  # finalizes that to `logical()` on the way out, so this still works.
+  expect_identical(vec_cast(TRUE, to = NA), TRUE)
+  expect_identical(vec_cast_common(TRUE, .to = NA), list(TRUE))
+
+  # `vec_cast()` itself does not call `vec_ptype()` and does not finalize,
+  # so this stays <unspecified> and the cast fails
+  # (this behavior is questionable, but is very much an edge case)
+  expect_snapshot(error = TRUE, {
+    vec_cast(TRUE, to = unspecified(1))
+  })
+
+  # `vec_cast_common()` calls `vec_ptype_common()`, which always finalises,
+  # so this technically works (but again, it is an edge case)
+  expect_identical(vec_cast_common(TRUE, .to = unspecified(1)), list(TRUE))
+})

tests/testthat/test-type.R

@@ -304,3 +304,32 @@ test_that("vec_ptype_common() handles spliced names consistently (#1570)", {
     )
   })
 })
+
+test_that("vec_ptype() and vec_ptype2() don't finalize their output", {
+  expect_identical(vec_ptype(NA), unspecified())
+  expect_identical(vec_ptype2(NA, NA), unspecified())
+  expect_identical(vec_ptype2(NA, NULL), unspecified())
+  expect_identical(vec_ptype2(NULL, NA), unspecified())
+
+  expect_identical(vec_ptype(unspecified()), unspecified())
+  expect_identical(vec_ptype2(unspecified(), unspecified()), unspecified())
+  expect_identical(vec_ptype2(unspecified(), NULL), unspecified())
+  expect_identical(vec_ptype2(NULL, unspecified()), unspecified())
+})
+
+test_that("vec_ptype_common() always finalizes its output (#2099)", {
+  expect_identical(vec_ptype_common(NA), logical())
+  expect_identical(vec_ptype_common(NA, NA), logical())
+  expect_identical(vec_ptype_common(unspecified(1)), logical())
+  expect_identical(vec_ptype_common(unspecified(1), unspecified(1)), logical())
+
+  # Even explicit `.ptype`s
+  expect_identical(
+    vec_ptype_common(.ptype = NA),
+    logical()
+  )
+  expect_identical(
+    vec_ptype_common(.ptype = unspecified(1)),
+    logical()
+  )
+})