refactor(typval)!: remove distinction of binary and nonbinary strings

This is a breaking change which will make refactor of typval and shada
code a lot easier. In particular, code that would use or check for
v:msgpack_types.binary in the wild would be broken. This appears to be
rarely used in existing plugins.

Also some cases where v:msgpack_type.string would be used to represent a
binary string of "string" type, we use a BLOB instead, which is
vimscripts native type for binary blobs, and already was used for BIN
formats when necessary.

msgpackdump(msgpackparse(data)) no longer preserves the distinction
of BIN and STR strings. This is very common behavior for
language-specific msgpack bindings. Nvim uses msgpack as a tool to
serialize its data. Nvim is not a tool to bit-perfectly manipulate
arbitrary msgpack data out in the wild.

The changed tests should indicate how behavior changes in various edge
cases.

8 files changed, 28 insertions, 85 deletions

diff --git a/src/nvim/api/private/converter.c b/src/nvim/api/private/converter.c
index 0a4daf1dae..a1c5816a49 100644
--- a/src/nvim/api/private/converter.c
+++ b/src/nvim/api/private/converter.c
@@ -7,7 +7,9 @@
 #include "nvim/api/private/converter.h"
 #include "nvim/api/private/defs.h"
 #include "nvim/api/private/helpers.h"
+#include "nvim/ascii_defs.h"
 #include "nvim/assert_defs.h"
+#include "nvim/eval/decode.h"
 #include "nvim/eval/typval.h"
 #include "nvim/eval/typval_defs.h"
 #include "nvim/eval/userfunc.h"
@@ -302,15 +304,11 @@ void object_to_vim_take_luaref(Object *obj, typval_T *tv, bool take_luaref, Erro
     tv->vval.v_float = obj->data.floating;
     break;
 
-  case kObjectTypeString:
-    tv->v_type = VAR_STRING;
-    if (obj->data.string.data == NULL) {
-      tv->vval.v_string = NULL;
-    } else {
-      tv->vval.v_string = xmemdupz(obj->data.string.data,
-                                   obj->data.string.size);
-    }
+  case kObjectTypeString: {
+    String s = obj->data.string;
+    *tv = decode_string(s.data, s.size, false, false);
     break;
+  }
 
   case kObjectTypeArray: {
     list_T *const list = tv_list_alloc((ptrdiff_t)obj->data.array.size);
diff --git a/src/nvim/eval.c b/src/nvim/eval.c
index d52e10f61b..b541af23af 100644
--- a/src/nvim/eval.c
+++ b/src/nvim/eval.c
@@ -331,7 +331,6 @@ static const char *const msgpack_type_names[] = {
   [kMPInteger] = "integer",
   [kMPFloat] = "float",
   [kMPString] = "string",
-  [kMPBinary] = "binary",
   [kMPArray] = "array",
   [kMPMap] = "map",
   [kMPExt] = "ext",
@@ -342,7 +341,6 @@ const list_T *eval_msgpack_type_lists[] = {
   [kMPInteger] = NULL,
   [kMPFloat] = NULL,
   [kMPString] = NULL,
-  [kMPBinary] = NULL,
   [kMPArray] = NULL,
   [kMPMap] = NULL,
   [kMPExt] = NULL,
diff --git a/src/nvim/eval.lua b/src/nvim/eval.lua
index 79a62874a4..82d695d82b 100644
--- a/src/nvim/eval.lua
+++ b/src/nvim/eval.lua
@@ -7444,12 +7444,7 @@ M.funcs = {
       	   C parser does not support such values.
       float	|Float|. This value cannot possibly appear in
       	|msgpackparse()| output.
-      string	|readfile()|-style list of strings. This value will
-      	appear in |msgpackparse()| output if string contains
-      	zero byte or if string is a mapping key and mapping is
-      	being represented as special dictionary for other
-      	reasons.
-      binary	|String|, or |Blob| if binary string contains zero
+      string	|String|, or |Blob| if binary string contains zero
       	byte. This value cannot appear in |msgpackparse()|
       	output since blobs were introduced.
       array	|List|. This value cannot appear in |msgpackparse()|
diff --git a/src/nvim/eval/decode.c b/src/nvim/eval/decode.c
index d7df7bb150..13cd3274dd 100644
--- a/src/nvim/eval/decode.c
+++ b/src/nvim/eval/decode.c
@@ -247,45 +247,29 @@ list_T *decode_create_map_special_dict(typval_T *const ret_tv, const ptrdiff_t l
 ///
 /// @param[in]  s  String to decode.
 /// @param[in]  len  String length.
-/// @param[in]  hasnul  Whether string has NUL byte, not or it was not yet
-///                     determined.
-/// @param[in]  binary  Determines decode type if string has NUL bytes.
-///                     If true convert string to VAR_BLOB, otherwise to the
-///                     kMPString special type.
+/// @param[in]  force_blob  whether string always should be decoded as a blob,
+///                         or only when embedded NUL bytes were present
 /// @param[in]  s_allocated  If true, then `s` was allocated and can be saved in
 ///                          a returned structure. If it is not saved there, it
 ///                          will be freed.
 ///
 /// @return Decoded string.
-typval_T decode_string(const char *const s, const size_t len, const TriState hasnul,
-                       const bool binary, const bool s_allocated)
+typval_T decode_string(const char *const s, const size_t len, bool force_blob,
+                       const bool s_allocated)
   FUNC_ATTR_WARN_UNUSED_RESULT
 {
   assert(s != NULL || len == 0);
-  const bool really_hasnul = (hasnul == kNone
-                              ? ((s != NULL) && (memchr(s, NUL, len) != NULL))
-                              : (bool)hasnul);
-  if (really_hasnul) {
+  const bool use_blob = force_blob || ((s != NULL) && (memchr(s, NUL, len) != NULL));
+  if (use_blob) {
     typval_T tv;
     tv.v_lock = VAR_UNLOCKED;
-    if (binary) {
-      tv_blob_alloc_ret(&tv);
-      ga_concat_len(&tv.vval.v_blob->bv_ga, s, len);
+    blob_T *b = tv_blob_alloc_ret(&tv);
+    if (s_allocated) {
+      b->bv_ga.ga_data = (void *)s;
+      b->bv_ga.ga_len = (int)len;
+      b->bv_ga.ga_maxlen = (int)len;
     } else {
-      list_T *const list = tv_list_alloc(kListLenMayKnow);
-      tv_list_ref(list);
-      create_special_dict(&tv, kMPString,
-                          (typval_T){ .v_type = VAR_LIST,
-                                      .v_lock = VAR_UNLOCKED,
-                                      .vval = { .v_list = list } });
-      const int elw_ret = encode_list_write((void *)list, s, len);
-      if (s_allocated) {
-        xfree((void *)s);
-      }
-      if (elw_ret == -1) {
-        tv_clear(&tv);
-        return (typval_T) { .v_type = VAR_UNKNOWN, .v_lock = VAR_UNLOCKED };
-      }
+      ga_concat_len(&b->bv_ga, s, len);
     }
     return tv;
   }
@@ -405,7 +389,6 @@ static inline int parse_json_string(const char *const buf, const size_t buf_len,
   char *str = xmalloc(len + 1);
   int fst_in_pair = 0;
   char *str_end = str;
-  bool hasnul = false;
 #define PUT_FST_IN_PAIR(fst_in_pair, str_end) \
   do { \
     if ((fst_in_pair) != 0) { \
@@ -426,9 +409,6 @@ static inline int parse_json_string(const char *const buf, const size_t buf_len,
         uvarnumber_T ch;
         vim_str2nr(ubuf, NULL, NULL,
                    STR2NR_HEX | STR2NR_FORCE, NULL, &ch, 4, true, NULL);
-        if (ch == 0) {
-          hasnul = true;
-        }
         if (SURROGATE_HI_START <= ch && ch <= SURROGATE_HI_END) {
           PUT_FST_IN_PAIR(fst_in_pair, str_end);
           fst_in_pair = (int)ch;
@@ -476,10 +456,7 @@ static inline int parse_json_string(const char *const buf, const size_t buf_len,
   PUT_FST_IN_PAIR(fst_in_pair, str_end);
 #undef PUT_FST_IN_PAIR
   *str_end = NUL;
-  typval_T obj = decode_string(str, (size_t)(str_end - str), hasnul ? kTrue : kFalse, false, true);
-  if (obj.v_type == VAR_UNKNOWN) {
-    goto parse_json_string_fail;
-  }
+  typval_T obj = decode_string(str, (size_t)(str_end - str), false, true);
   POP(obj, obj.v_type != VAR_STRING);
   goto parse_json_string_ret;
 parse_json_string_fail:
@@ -982,18 +959,8 @@ int msgpack_to_vim(const msgpack_object mobj, typval_T *const rettv)
     };
     break;
   case MSGPACK_OBJECT_STR:
-    *rettv = decode_string(mobj.via.bin.ptr, mobj.via.bin.size, kTrue, false,
-                           false);
-    if (rettv->v_type == VAR_UNKNOWN) {
-      return FAIL;
-    }
-    break;
   case MSGPACK_OBJECT_BIN:
-    *rettv = decode_string(mobj.via.bin.ptr, mobj.via.bin.size, kNone, true,
-                           false);
-    if (rettv->v_type == VAR_UNKNOWN) {
-      return FAIL;
-    }
+    *rettv = decode_string(mobj.via.bin.ptr, mobj.via.bin.size, false, false);
     break;
   case MSGPACK_OBJECT_ARRAY: {
     list_T *const list = tv_list_alloc((ptrdiff_t)mobj.via.array.size);
@@ -1016,7 +983,8 @@ int msgpack_to_vim(const msgpack_object mobj, typval_T *const rettv)
   }
   case MSGPACK_OBJECT_MAP: {
     for (size_t i = 0; i < mobj.via.map.size; i++) {
-      if (mobj.via.map.ptr[i].key.type != MSGPACK_OBJECT_STR
+      if ((mobj.via.map.ptr[i].key.type != MSGPACK_OBJECT_STR
+           && mobj.via.map.ptr[i].key.type != MSGPACK_OBJECT_BIN)
           || mobj.via.map.ptr[i].key.via.str.size == 0
           || memchr(mobj.via.map.ptr[i].key.via.str.ptr, NUL,
                     mobj.via.map.ptr[i].key.via.str.size) != NULL) {
diff --git a/src/nvim/eval/encode.c b/src/nvim/eval/encode.c
index eb70b0534d..383cf9f1bd 100644
--- a/src/nvim/eval/encode.c
+++ b/src/nvim/eval/encode.c
@@ -776,8 +776,7 @@ bool encode_check_json_key(const typval_T *const tv)
   const dictitem_T *val_di;
   if ((type_di = tv_dict_find(spdict, S_LEN("_TYPE"))) == NULL
       || type_di->di_tv.v_type != VAR_LIST
-      || (type_di->di_tv.vval.v_list != eval_msgpack_type_lists[kMPString]
-          && type_di->di_tv.vval.v_list != eval_msgpack_type_lists[kMPBinary])
+      || type_di->di_tv.vval.v_list != eval_msgpack_type_lists[kMPString]
       || (val_di = tv_dict_find(spdict, S_LEN("_VAL"))) == NULL
       || val_di->di_tv.v_type != VAR_LIST) {
     return false;
diff --git a/src/nvim/eval/typval_encode.c.h b/src/nvim/eval/typval_encode.c.h
index afee6dab18..3a78fc5afd 100644
--- a/src/nvim/eval/typval_encode.c.h
+++ b/src/nvim/eval/typval_encode.c.h
@@ -501,9 +501,7 @@ static int TYPVAL_ENCODE_CONVERT_ONE_VALUE(
         }
         TYPVAL_ENCODE_CONV_FLOAT(tv, val_di->di_tv.vval.v_float);
         break;
-      case kMPString:
-      case kMPBinary: {
-        const bool is_string = ((MessagePackType)i == kMPString);
+      case kMPString: {
         if (val_di->di_tv.v_type != VAR_LIST) {
           goto _convert_one_value_regular_dict;
         }
@@ -513,11 +511,7 @@ static int TYPVAL_ENCODE_CONVERT_ONE_VALUE(
                                     &buf)) {
           goto _convert_one_value_regular_dict;
         }
-        if (is_string) {
-          TYPVAL_ENCODE_CONV_STR_STRING(tv, buf, len);
-        } else {
-          TYPVAL_ENCODE_CONV_STRING(tv, buf, len);
-        }
+        TYPVAL_ENCODE_CONV_STR_STRING(tv, buf, len);
         xfree(buf);
         break;
       }
diff --git a/src/nvim/eval_defs.h b/src/nvim/eval_defs.h
index 4bbebb14f5..7adea6dfa9 100644
--- a/src/nvim/eval_defs.h
+++ b/src/nvim/eval_defs.h
@@ -9,7 +9,6 @@ typedef enum {
   kMPInteger,
   kMPFloat,
   kMPString,
-  kMPBinary,
   kMPArray,
   kMPMap,
   kMPExt,
diff --git a/src/nvim/lua/converter.c b/src/nvim/lua/converter.c
index 1b1794e244..c8ad6606bf 100644
--- a/src/nvim/lua/converter.c
+++ b/src/nvim/lua/converter.c
@@ -219,12 +219,7 @@ bool nlua_pop_typval(lua_State *lstate, typval_T *ret_tv)
           if (cur.special) {
             list_T *const kv_pair = tv_list_alloc(2);
 
-            typval_T s_tv = decode_string(s, len, kTrue, false, false);
-            if (s_tv.v_type == VAR_UNKNOWN) {
-              ret = false;
-              tv_list_unref(kv_pair);
-              continue;
-            }
+            typval_T s_tv = decode_string(s, len, true, false);
             tv_list_append_owned_tv(kv_pair, s_tv);
 
             // Value: not populated yet, need to create list item to push.
@@ -280,10 +275,7 @@ bool nlua_pop_typval(lua_State *lstate, typval_T *ret_tv)
     case LUA_TSTRING: {
       size_t len;
       const char *s = lua_tolstring(lstate, -1, &len);
-      *cur.tv = decode_string(s, len, kNone, true, false);
-      if (cur.tv->v_type == VAR_UNKNOWN) {
-        ret = false;
-      }
+      *cur.tv = decode_string(s, len, false, false);
       break;
     }
     case LUA_TNUMBER: {