From b511603643f46408e535ef38665ff7b5223bfc79 Mon Sep 17 00:00:00 2001
From: Nash Delcamp <107637296+nash-delcamp-slp@users.noreply.github.com>
Date: Tue, 20 Aug 2024 15:48:04 -0400
Subject: [PATCH] Make type.character error for NA_character_ (#566)

Fixes #546

---------

Co-authored-by: Hadley Wickham <hadley@posit.co>
---
 NEWS.md                            |  1 +
 R/modifiers.R                      |  9 ++++++++-
 tests/testthat/_snaps/modifiers.md | 15 ++++++++++++++-
 tests/testthat/_snaps/split.md     |  2 +-
 tests/testthat/test-modifiers.R    |  7 +++++++
 5 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index c0f82abc..6aae9977 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,5 +1,6 @@
 # stringr (development version)
 
+* `str_*` now errors if `pattern` includes any `NA`s (@nash-delcamp-slp, #546).
 * `str_view()` now displays a message when called with a zero-length character
   vector (@LouisMPenrod, #497).
 * Adds `[[.stringr_pattern` method to go along with existing `[.stringr_pattern`
diff --git a/R/modifiers.R b/R/modifiers.R
index 7783b807..f5afb6f4 100644
--- a/R/modifiers.R
+++ b/R/modifiers.R
@@ -202,6 +202,13 @@ type.stringr_fixed <- function(x, error_call = caller_env()) {
 }
 #' @export
 type.character <- function(x, error_call = caller_env()) {
+  if (any(is.na(x))) {
+    cli::cli_abort(
+      tr_("{.arg pattern} must be a character vector that does not contain NAs."),
+      call = error_call
+    )
+  }
+
   if (identical(x, "")) "empty" else "regex"
 }
 
@@ -213,7 +220,7 @@ type.default <- function(x, error_call = caller_env()) {
   }
 
   cli::cli_abort(
-    tr_("{.arg pattern} must be a string, not {.obj_type_friendly {x}}."),
+    tr_("{.arg pattern} must be a character vector, not {.obj_type_friendly {x}}."),
     call = error_call
   )
 }
diff --git a/tests/testthat/_snaps/modifiers.md b/tests/testthat/_snaps/modifiers.md
index 5d5e3307..67ae93f8 100644
--- a/tests/testthat/_snaps/modifiers.md
+++ b/tests/testthat/_snaps/modifiers.md
@@ -22,5 +22,18 @@
       type(1:3)
     Condition
       Error:
-      ! `pattern` must be a string, not an integer vector.
+      ! `pattern` must be a character vector, not an integer vector.
+
+# useful errors for NAs
+
+    Code
+      type(NA)
+    Condition
+      Error:
+      ! `pattern` must be a character vector, not `NA`.
+    Code
+      type(c("a", "b", NA_character_, "c"))
+    Condition
+      Error:
+      ! `pattern` must be a character vector that does not contain NAs.
 
diff --git a/tests/testthat/_snaps/split.md b/tests/testthat/_snaps/split.md
index 574b9f8c..6b00f942 100644
--- a/tests/testthat/_snaps/split.md
+++ b/tests/testthat/_snaps/split.md
@@ -9,7 +9,7 @@
       str_split("x", 1)
     Condition
       Error in `str_split()`:
-      ! `pattern` must be a string, not a number.
+      ! `pattern` must be a character vector, not a number.
     Code
       str_split("x", "x", n = 0)
     Condition
diff --git a/tests/testthat/test-modifiers.R b/tests/testthat/test-modifiers.R
index aa1624c8..74e2765e 100644
--- a/tests/testthat/test-modifiers.R
+++ b/tests/testthat/test-modifiers.R
@@ -37,6 +37,13 @@ test_that("subsetting preserves class and options", {
   expect_equal(x[], x)
 })
 
+test_that("useful errors for NAs", {
+  expect_snapshot(error = TRUE, {
+    type(NA)
+    type(c("a", "b", NA_character_, "c"))
+  })
+})
+
 test_that("stringr_pattern methods", {
   ex <- coll(c("foo", "bar"))
   expect_true(inherits(ex[1], "stringr_pattern"))