vim-patch:8.1.0619: :echomsg and :echoerr do not handle List and Dict

Problem:    :echomsg and :echoerr do not handle List and Dict like :echo does.
            (Daniel Hahler)
Solution:   Be more tolerant about the expression result type.
https://github.com/vim/vim/commit/461a7fcfce3cd6414f990037e6468af3b5ccf119

Add lua functional tests for :echo,:echon,:echomsg,:echoerr
because nvim did not port "test_" functions from Vim
that modify internal state.

Testing :echoerr via try/catch is sufficient.

5 files changed, 178 insertions, 54 deletions

diff --git a/runtime/doc/eval.txt b/runtime/doc/eval.txt
index 113a92d5e4..ce0218f1dd 100644
--- a/runtime/doc/eval.txt
+++ b/runtime/doc/eval.txt
@@ -10370,8 +10370,8 @@ text...
 			The parsing works slightly different from |:echo|,
 			more like |:execute|.  All the expressions are first
 			evaluated and concatenated before echoing anything.
-			The expressions must evaluate to a Number or String, a
-			Dictionary or List causes an error.
+			If expressions does not evaluate to a Number or
+			String, string() is used to turn it into a string.
 			Uses the highlighting set by the |:echohl| command.
 			Example: >
 		:echomsg "It's a Zizzer Zazzer Zuzz, as you can plainly see."
@@ -10382,7 +10382,7 @@ text...
 			message in the |message-history|.  When used in a
 			script or function the line number will be added.
 			Spaces are placed between the arguments as with the
-			:echo command.  When used inside a try conditional,
+			|:echomsg| command.  When used inside a try conditional,
 			the message is raised as an error exception instead
 			(see |try-echoerr|).
 			Example: >
diff --git a/src/nvim/eval.c b/src/nvim/eval.c
index 74a5edc0df..f5c5ef9e97 100644
--- a/src/nvim/eval.c
+++ b/src/nvim/eval.c
@@ -9459,6 +9459,27 @@ void set_selfdict(typval_T *rettv, dict_T *selfdict)
   }
 }
 
+// Turn a typeval into a string.  Similar to tv_get_string_buf() but uses
+// string() on Dict, List, etc.
+static const char *tv_stringify(typval_T *varp, char *buf)
+  FUNC_ATTR_NONNULL_ALL
+{
+  if (varp->v_type == VAR_LIST
+      || varp->v_type == VAR_DICT
+      || varp->v_type == VAR_FUNC
+      || varp->v_type == VAR_PARTIAL
+      || varp->v_type == VAR_FLOAT) {
+    typval_T tmp;
+
+    f_string(varp, &tmp, NULL);
+    const char *const res = tv_get_string_buf(&tmp, buf);
+    tv_clear(varp);
+    *varp = tmp;
+    return res;
+  }
+  return tv_get_string_buf(varp, buf);
+}
+
 // Find variable "name" in the list of variables.
 // Return a pointer to it if found, NULL if not found.
 // Careful: "a:0" variables don't have a name.
@@ -10349,7 +10370,10 @@ void ex_execute(exarg_T *eap)
     }
 
     if (!eap->skip) {
-      const char *const argstr = tv_get_string(&rettv);
+      char buf[NUMBUFLEN];
+      const char *const argstr = eap->cmdidx == CMD_execute
+        ? tv_get_string_buf(&rettv, buf)
+        : tv_stringify(&rettv, buf);
       const size_t len = strlen(argstr);
       ga_grow(&ga, len + 2);
       if (!GA_EMPTY(&ga)) {
diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c
index bd57cec794..be8e35b1de 100644
--- a/src/nvim/eval/funcs.c
+++ b/src/nvim/eval/funcs.c
@@ -9558,7 +9558,7 @@ static void f_stridx(typval_T *argvars, typval_T *rettv, FunPtr fptr)
 /*
  * "string()" function
  */
-static void f_string(typval_T *argvars, typval_T *rettv, FunPtr fptr)
+void f_string(typval_T *argvars, typval_T *rettv, FunPtr fptr)
 {
   rettv->v_type = VAR_STRING;
   rettv->vval.v_string = (char_u *)encode_tv2string(&argvars[0], NULL);
diff --git a/src/nvim/testdir/test_messages.vim b/src/nvim/testdir/test_messages.vim
index 265dee66ce..aad21c002f 100644
--- a/src/nvim/testdir/test_messages.vim
+++ b/src/nvim/testdir/test_messages.vim
@@ -1,4 +1,4 @@
-" Tests for :messages
+" Tests for :messages, :echomsg, :echoerr
 
 function Test_messages()
   let oldmore = &more
@@ -65,6 +65,35 @@ func Test_message_completion()
   call assert_equal('"message clear', @:)
 endfunc
 
+func Test_echomsg()
+  call assert_equal("\nhello", execute(':echomsg "hello"'))
+  call assert_equal("\n", execute(':echomsg ""'))
+  call assert_equal("\n12345", execute(':echomsg 12345'))
+  call assert_equal("\n[]", execute(':echomsg []'))
+  call assert_equal("\n[1, 2, 3]", execute(':echomsg [1, 2, 3]'))
+  call assert_equal("\n{}", execute(':echomsg {}'))
+  call assert_equal("\n{'a': 1, 'b': 2}", execute(':echomsg {"a": 1, "b": 2}'))
+  if has('float')
+    call assert_equal("\n1.23", execute(':echomsg 1.23'))
+  endif
+  call assert_match("function('<lambda>\\d*')", execute(':echomsg {-> 1234}'))
+endfunc
+
+func Test_echoerr()
+  throw 'skipped: Nvim does not support test_ignore_error()'
+  call test_ignore_error('IgNoRe')
+  call assert_equal("\nIgNoRe hello", execute(':echoerr "IgNoRe hello"'))
+  call assert_equal("\n12345 IgNoRe", execute(':echoerr 12345 "IgNoRe"'))
+  call assert_equal("\n[1, 2, 'IgNoRe']", execute(':echoerr [1, 2, "IgNoRe"]'))
+  call assert_equal("\n{'IgNoRe': 2, 'a': 1}", execute(':echoerr {"a": 1, "IgNoRe": 2}'))
+  if has('float')
+    call assert_equal("\n1.23 IgNoRe", execute(':echoerr 1.23 "IgNoRe"'))
+  endif
+  call test_ignore_error('<lambda>')
+  call assert_match("function('<lambda>\\d*')", execute(':echoerr {-> 1234}'))
+  call test_ignore_error('RESET')
+endfunc
+
 func Test_echospace()
   set noruler noshowcmd laststatus=1
   call assert_equal(&columns - 1, v:echospace)
diff --git a/test/functional/ex_cmds/echo_spec.lua b/test/functional/ex_cmds/echo_spec.lua
index 10c7230896..408ce52b8c 100644
--- a/test/functional/ex_cmds/echo_spec.lua
+++ b/test/functional/ex_cmds/echo_spec.lua
@@ -11,31 +11,57 @@ local dedent = helpers.dedent
 local command = helpers.command
 local exc_exec = helpers.exc_exec
 local redir_exec = helpers.redir_exec
+local matches = helpers.matches
+
+describe(':echo :echon :echomsg :echoerr', function()
+  local fn_tbl = {'String', 'StringN', 'StringMsg', 'StringErr'}
+  local function assert_same_echo_dump(expected, input, use_eval)
+    for _,v in pairs(fn_tbl) do
+      eq(expected, use_eval and eval(v..'('..input..')') or funcs[v](input))
+    end
+  end
+  local function assert_matches_echo_dump(expected, input, use_eval)
+    for _,v in pairs(fn_tbl) do
+      matches(expected, use_eval and eval(v..'('..input..')') or funcs[v](input))
+    end
+  end
 
-describe(':echo', function()
   before_each(function()
     clear()
     source([[
       function String(s)
         return execute('echo a:s')[1:]
       endfunction
+      function StringMsg(s)
+        return execute('echomsg a:s')[1:]
+      endfunction
+      function StringN(s)
+        return execute('echon a:s')
+      endfunction
+      function StringErr(s)
+        try
+          execute 'echoerr a:s'
+        catch
+          return substitute(v:exception, '^Vim(echoerr):', '', '')
+        endtry
+      endfunction
     ]])
   end)
 
   describe('used to represent floating-point values', function()
     it('dumps NaN values', function()
-      eq('str2float(\'nan\')', eval('String(str2float(\'nan\'))'))
+      assert_same_echo_dump("str2float('nan')", "str2float('nan')", true)
     end)
 
     it('dumps infinite values', function()
-      eq('str2float(\'inf\')', eval('String(str2float(\'inf\'))'))
-      eq('-str2float(\'inf\')', eval('String(str2float(\'-inf\'))'))
+      assert_same_echo_dump("str2float('inf')", "str2float('inf')", true)
+      assert_same_echo_dump("-str2float('inf')", "str2float('-inf')", true)
     end)
 
     it('dumps regular values', function()
-      eq('1.5', funcs.String(1.5))
-      eq('1.56e-20', funcs.String(1.56000e-020))
-      eq('0.0', eval('String(0.0)'))
+      assert_same_echo_dump('1.5', 1.5)
+      assert_same_echo_dump('1.56e-20', 1.56000e-020)
+      assert_same_echo_dump('0.0', '0.0', true)
     end)
 
     it('dumps special v: values', function()
@@ -45,69 +71,81 @@ describe(':echo', function()
       eq('v:true', funcs.String(true))
       eq('v:false', funcs.String(false))
       eq('v:null', funcs.String(NIL))
+      eq('true', eval('StringMsg(v:true)'))
+      eq('false', eval('StringMsg(v:false)'))
+      eq('null', eval('StringMsg(v:null)'))
+      eq('true', funcs.StringMsg(true))
+      eq('false', funcs.StringMsg(false))
+      eq('null', funcs.StringMsg(NIL))
+      eq('true', eval('StringErr(v:true)'))
+      eq('false', eval('StringErr(v:false)'))
+      eq('null', eval('StringErr(v:null)'))
+      eq('true', funcs.StringErr(true))
+      eq('false', funcs.StringErr(false))
+      eq('null', funcs.StringErr(NIL))
     end)
 
     it('dumps values with at most six digits after the decimal point',
     function()
-      eq('1.234568e-20', funcs.String(1.23456789123456789123456789e-020))
-      eq('1.234568', funcs.String(1.23456789123456789123456789))
+      assert_same_echo_dump('1.234568e-20', 1.23456789123456789123456789e-020)
+      assert_same_echo_dump('1.234568', 1.23456789123456789123456789)
     end)
 
     it('dumps values with at most seven digits before the decimal point',
     function()
-      eq('1234567.891235', funcs.String(1234567.89123456789123456789))
-      eq('1.234568e7', funcs.String(12345678.9123456789123456789))
+      assert_same_echo_dump('1234567.891235', 1234567.89123456789123456789)
+      assert_same_echo_dump('1.234568e7', 12345678.9123456789123456789)
     end)
 
     it('dumps negative values', function()
-      eq('-1.5', funcs.String(-1.5))
-      eq('-1.56e-20', funcs.String(-1.56000e-020))
-      eq('-1.234568e-20', funcs.String(-1.23456789123456789123456789e-020))
-      eq('-1.234568', funcs.String(-1.23456789123456789123456789))
-      eq('-1234567.891235', funcs.String(-1234567.89123456789123456789))
-      eq('-1.234568e7', funcs.String(-12345678.9123456789123456789))
+      assert_same_echo_dump('-1.5', -1.5)
+      assert_same_echo_dump('-1.56e-20', -1.56000e-020)
+      assert_same_echo_dump('-1.234568e-20', -1.23456789123456789123456789e-020)
+      assert_same_echo_dump('-1.234568', -1.23456789123456789123456789)
+      assert_same_echo_dump('-1234567.891235', -1234567.89123456789123456789)
+      assert_same_echo_dump('-1.234568e7', -12345678.9123456789123456789)
     end)
   end)
 
   describe('used to represent numbers', function()
     it('dumps regular values', function()
-      eq('0', funcs.String(0))
-      eq('-1', funcs.String(-1))
-      eq('1', funcs.String(1))
+      assert_same_echo_dump('0', 0)
+      assert_same_echo_dump('-1', -1)
+      assert_same_echo_dump('1', 1)
     end)
 
     it('dumps large values', function()
-      eq('2147483647', funcs.String(2^31-1))
-      eq('-2147483648', funcs.String(-2^31))
+      assert_same_echo_dump('2147483647', 2^31-1)
+      assert_same_echo_dump('-2147483648', -2^31)
     end)
   end)
 
   describe('used to represent strings', function()
     it('dumps regular strings', function()
-      eq('test', funcs.String('test'))
+      assert_same_echo_dump('test', 'test')
     end)
 
     it('dumps empty strings', function()
-      eq('', funcs.String(''))
+      assert_same_echo_dump('', '')
     end)
 
-    it('dumps strings with \' inside', function()
-      eq('\'\'\'', funcs.String('\'\'\''))
-      eq('a\'b\'\'', funcs.String('a\'b\'\''))
-      eq('\'b\'\'d', funcs.String('\'b\'\'d'))
-      eq('a\'b\'c\'d', funcs.String('a\'b\'c\'d'))
+    it("dumps strings with ' inside", function()
+      assert_same_echo_dump("'''", "'''")
+      assert_same_echo_dump("a'b''", "a'b''")
+      assert_same_echo_dump("'b''d", "'b''d")
+      assert_same_echo_dump("a'b'c'd", "a'b'c'd")
     end)
 
     it('dumps NULL strings', function()
-      eq('', eval('String($XXX_UNEXISTENT_VAR_XXX)'))
+      assert_same_echo_dump('', '$XXX_UNEXISTENT_VAR_XXX', true)
     end)
 
     it('dumps NULL lists', function()
-      eq('[]', eval('String(v:_null_list)'))
+      assert_same_echo_dump('[]', 'v:_null_list', true)
     end)
 
     it('dumps NULL dictionaries', function()
-      eq('{}', eval('String(v:_null_dict)'))
+      assert_same_echo_dump('{}', 'v:_null_dict', true)
     end)
   end)
 
@@ -129,15 +167,27 @@ describe(':echo', function()
 
     it('dumps references to built-in functions', function()
       eq('function', eval('String(function("function"))'))
+      eq("function('function')", eval('StringMsg(function("function"))'))
+      eq("function('function')", eval('StringErr(function("function"))'))
     end)
 
     it('dumps references to user functions', function()
       eq('Test1', eval('String(function("Test1"))'))
       eq('g:Test3', eval('String(function("g:Test3"))'))
+      eq("function('Test1')", eval("StringMsg(function('Test1'))"))
+      eq("function('g:Test3')", eval("StringMsg(function('g:Test3'))"))
+      eq("function('Test1')", eval("StringErr(function('Test1'))"))
+      eq("function('g:Test3')", eval("StringErr(function('g:Test3'))"))
     end)
 
     it('dumps references to script functions', function()
       eq('<SNR>2_Test2', eval('String(Test2_f)'))
+      eq("function('<SNR>2_Test2')", eval('StringMsg(Test2_f)'))
+      eq("function('<SNR>2_Test2')", eval('StringErr(Test2_f)'))
+    end)
+
+    it('dump references to lambdas', function()
+      assert_matches_echo_dump("function%('<lambda>%d+'%)", '{-> 1234}', true)
     end)
 
     it('dumps partials with self referencing a partial', function()
@@ -156,19 +206,23 @@ describe(':echo', function()
     end)
 
     it('dumps automatically created partials', function()
-      eq('function(\'<SNR>2_Test2\', {\'f\': function(\'<SNR>2_Test2\')})',
-         eval('String({"f": Test2_f}.f)'))
-      eq('function(\'<SNR>2_Test2\', [1], {\'f\': function(\'<SNR>2_Test2\', [1])})',
-         eval('String({"f": function(Test2_f, [1])}.f)'))
+      assert_same_echo_dump(
+        "function('<SNR>2_Test2', {'f': function('<SNR>2_Test2')})",
+        '{"f": Test2_f}.f',
+        true)
+      assert_same_echo_dump(
+        "function('<SNR>2_Test2', [1], {'f': function('<SNR>2_Test2', [1])})",
+        '{"f": function(Test2_f, [1])}.f',
+        true)
     end)
 
     it('dumps manually created partials', function()
-      eq('function(\'Test3\', [1, 2], {})',
-         eval('String(function("Test3", [1, 2], {}))'))
-      eq('function(\'Test3\', {})',
-         eval('String(function("Test3", {}))'))
-      eq('function(\'Test3\', [1, 2])',
-         eval('String(function("Test3", [1, 2]))'))
+      assert_same_echo_dump("function('Test3', [1, 2], {})",
+                            "function('Test3', [1, 2], {})", true)
+      assert_same_echo_dump("function('Test3', [1, 2])",
+                            "function('Test3', [1, 2])", true)
+      assert_same_echo_dump("function('Test3', {})",
+                            "function('Test3', {})", true)
     end)
 
     it('does not crash or halt when dumping partials with reference cycles in self',
@@ -225,15 +279,19 @@ describe(':echo', function()
 
   describe('used to represent lists', function()
     it('dumps empty list', function()
-      eq('[]', funcs.String({}))
+      assert_same_echo_dump('[]', {})
+    end)
+
+    it('dumps non-empty list', function()
+      assert_same_echo_dump('[1, 2]', {1,2})
     end)
 
     it('dumps nested lists', function()
-      eq('[[[[[]]]]]', funcs.String({{{{{}}}}}))
+      assert_same_echo_dump('[[[[[]]]]]', {{{{{}}}}})
     end)
 
     it('dumps nested non-empty lists', function()
-      eq('[1, [[3, [[5], 4]], 2]]', funcs.String({1, {{3, {{5}, 4}}, 2}}))
+      assert_same_echo_dump('[1, [[3, [[5], 4]], 2]]', {1, {{3, {{5}, 4}}, 2}})
     end)
 
     it('does not error when dumping recursive lists', function()
@@ -252,18 +310,18 @@ describe(':echo', function()
 
   describe('used to represent dictionaries', function()
     it('dumps empty dictionary', function()
-      eq('{}', eval('String({})'))
+      assert_same_echo_dump('{}', '{}', true)
     end)
 
     it('dumps list with two same empty dictionaries, also in partials', function()
       command('let d = {}')
-      eq('[{}, {}]', eval('String([d, d])'))
+      assert_same_echo_dump('[{}, {}]', '[d, d]', true)
       eq('[function(\'tr\', {}), {}]', eval('String([function("tr", d), d])'))
       eq('[{}, function(\'tr\', {})]', eval('String([d, function("tr", d)])'))
     end)
 
     it('dumps non-empty dictionary', function()
-      eq('{\'t\'\'est\': 1}', funcs.String({['t\'est']=1}))
+      assert_same_echo_dump("{'t''est': 1}", {["t'est"]=1})
     end)
 
     it('does not error when dumping recursive dictionaries', function()
@@ -297,11 +355,20 @@ describe(':echo', function()
       eq('<8e>', funcs.String(chr(0x8e)))
       eq('<c2>', funcs.String(('«'):sub(1, 1)))
       eq('«', funcs.String(('«'):sub(1, 2)))
+
+      eq('<80>', funcs.StringMsg(chr(0x80)))
+      eq('<81>', funcs.StringMsg(chr(0x81)))
+      eq('<8e>', funcs.StringMsg(chr(0x8e)))
+      eq('<c2>', funcs.StringMsg(('«'):sub(1, 1)))
+      eq('«', funcs.StringMsg(('«'):sub(1, 2)))
     end)
     it('displays ASCII control characters using ^X notation', function()
       eq('^C', funcs.String(ctrl('c')))
       eq('^A', funcs.String(ctrl('a')))
       eq('^F', funcs.String(ctrl('f')))
+      eq('^C', funcs.StringMsg(ctrl('c')))
+      eq('^A', funcs.StringMsg(ctrl('a')))
+      eq('^F', funcs.StringMsg(ctrl('f')))
     end)
     it('prints CR, NL and tab as-is', function()
       eq('\n', funcs.String('\n'))
@@ -311,11 +378,15 @@ describe(':echo', function()
     it('prints non-printable UTF-8 in <> notation', function()
       -- SINGLE SHIFT TWO, unicode control
       eq('<8e>', funcs.String(funcs.nr2char(0x8E)))
+      eq('<8e>', funcs.StringMsg(funcs.nr2char(0x8E)))
       -- Surrogate pair: U+1F0A0 PLAYING CARD BACK is represented in UTF-16 as
       -- 0xD83C 0xDCA0. This is not valid in UTF-8.
       eq('<d83c>', funcs.String(funcs.nr2char(0xD83C)))
       eq('<dca0>', funcs.String(funcs.nr2char(0xDCA0)))
       eq('<d83c><dca0>', funcs.String(funcs.nr2char(0xD83C) .. funcs.nr2char(0xDCA0)))
+      eq('<d83c>', funcs.StringMsg(funcs.nr2char(0xD83C)))
+      eq('<dca0>', funcs.StringMsg(funcs.nr2char(0xDCA0)))
+      eq('<d83c><dca0>', funcs.StringMsg(funcs.nr2char(0xD83C) .. funcs.nr2char(0xDCA0)))
     end)
   end)
 end)