patch 8.2.0626: Vim9: wrong syntax of function in Vim9 script

Problem:    Vim9: wrong syntax of function in Vim9 script.
Solution:   Give error for missing space. Implement :echomsg and :echoerr.
            (closes #5670)

src/eval.c

@@ -34,8 +34,6 @@ static char *e_float_as_string = N_("E806: using Float as a String");
  */
 static int current_copyID = 0;
 
-static int echo_attr = 0;   // attributes used for ":echo"
-
 /*
  * Info used by a ":for" loop.
  */
@@ -6145,7 +6143,6 @@ ex_execute(exarg_T *eap)
     char_u	*p;
     garray_T	ga;
     int		len;
-    int		save_did_emsg;
 
     ga_init2(&ga, 1, 80);
 
@@ -6213,8 +6210,9 @@ ex_execute(exarg_T *eap)
 	}
 	else if (eap->cmdidx == CMD_echoerr)
 	{
+	    int		save_did_emsg = did_emsg;
+
 	    // We don't want to abort following commands, restore did_emsg.
-	    save_did_emsg = did_emsg;
 	    emsg(ga.ga_data);
 	    if (!force_abort)
 		did_emsg = save_did_emsg;

src/globals.h

@@ -1750,6 +1750,10 @@ EXTERN char e_endif_without_if[] INIT(= N_("E580: :endif without :if"));
 EXTERN char e_continue[]	INIT(= N_("E586: :continue without :while or :for"));
 EXTERN char e_break[]		INIT(= N_("E587: :break without :while or :for"));
 EXTERN char e_nowhitespace[]	INIT(= N_("E274: No white space allowed before parenthesis"));
+EXTERN char e_white_both[]	INIT(= N_("E1004: white space required before and after '%s'"));
+EXTERN char e_white_after[]	INIT(= N_("E1069: white space required after '%s'"));
+EXTERN char e_no_white_before[] INIT(= N_("E1068: No white space allowed before '%s'"));
+
 EXTERN char e_lock_unlock[]	INIT(= N_("E940: Cannot lock or unlock variable %s"));
 #endif
 
@@ -1819,6 +1823,8 @@ EXTERN proftime_T bevalexpr_due;
 #ifdef FEAT_EVAL
 EXTERN time_T time_for_testing INIT(= 0);
 
+EXTERN int echo_attr INIT(= 0);   // attributes used for ":echo"
+
 // Abort conversion to string after a recursion error.
 EXTERN int  did_echo_string_emsg INIT(= FALSE);
 

src/testdir/test_vim9_disassemble.vim

@@ -919,6 +919,28 @@ def Test_disassemble_execute()
         res)
 enddef
 
+def s:Echomsg()
+  echomsg 'some' 'message'
+  echoerr 'went' .. 'wrong'
+enddef
+
+def Test_disassemble_echomsg()
+  let res = execute('disass s:Echomsg')
+  assert_match('\<SNR>\d*_Echomsg.*' ..
+        "echomsg 'some' 'message'.*" ..
+        '\d PUSHS "some".*' ..
+        '\d PUSHS "message".*' ..
+        '\d ECHOMSG 2.*' ..
+        "echoerr 'went' .. 'wrong'.*" ..
+        '\d PUSHS "went".*' ..
+        '\d PUSHS "wrong".*' ..
+        '\d CONCAT.*' ..
+        '\d ECHOERR 1.*' ..
+        '\d PUSHNR 0.*' ..
+        '\d RETURN',
+        res)
+enddef
+
 def SomeStringArg(arg: string)
   echo arg
 enddef

src/testdir/test_vim9_func.vim

@@ -259,6 +259,7 @@ enddef
 def Test_arg_type_wrong()
   CheckScriptFailure(['def Func3(items: list)', 'echo "a"', 'enddef'], 'E1008: Missing <type>')
   CheckScriptFailure(['def Func4(...)', 'echo "a"', 'enddef'], 'E1055: Missing name after ...')
+  CheckScriptFailure(['def Func5(items:string)', 'echo "a"'], 'E1069:')
   CheckScriptFailure(['def Func5(items)', 'echo "a"'], 'E1077:')
 enddef
 
@@ -390,6 +391,20 @@ def Test_redef_failure()
   delfunc! Func2
 enddef
 
+def Test_vim9script_func()
+  let lines =<< trim END
+    vim9script
+    func Func(arg)
+      echo a:arg
+    endfunc
+    Func('text')
+  END
+  writefile(lines, 'XVim9Func')
+  so XVim9Func
+
+  delete('XVim9Func')
+enddef
+
 " Test for internal functions returning different types
 func Test_InternalFuncRetType()
   let lines =<< trim END

src/testdir/test_vim9_script.vim

@@ -54,6 +54,10 @@ def Test_assignment()
   let dict4: dict<any> = #{one: 1, two: '2'}
   let dict5: dict<blob> = #{one: 0z01, tw: 0z02}
 
+  call CheckDefFailure(['let x:string'], 'E1069:')
+  call CheckDefFailure(['let x:string = "x"'], 'E1069:')
+  call CheckDefFailure(['let a:string = "x"'], 'E1082:')
+
   let a: number = 6
   assert_equal(6, a)
 
@@ -976,6 +980,20 @@ def Test_echo_cmd()
   call CheckDefFailure(['echo "xxx"# comment'], 'E488:')
 enddef
 
+def Test_echomsg_cmd()
+  echomsg 'some' 'more' # comment
+  assert_match('^some more$', Screenline(&lines))
+  echo 'clear'
+  1messages
+  assert_match('^some more$', Screenline(&lines))
+
+  call CheckDefFailure(['echomsg "xxx"# comment'], 'E488:')
+enddef
+
+def Test_echoerr_cmd()
+  # TODO: write this test
+enddef
+
 def Test_for_outside_of_function()
   let lines =<< trim END
     vim9script

src/userfunc.c

@@ -128,7 +128,13 @@ one_function_arg(char_u *arg, garray_T *newargs, garray_T *argtypes, int skip)
 	}
 	if (*p == ':')
 	{
-	    type = skipwhite(p + 1);
+	    ++p;
+	    if (!VIM_ISWHITE(*p))
+	    {
+		semsg(_(e_white_after), ":");
+		return arg;
+	    }
+	    type = skipwhite(p);
 	    p = skip_type(type);
 	    type = vim_strnsave(type, p - type);
 	}

src/version.c

@@ -746,6 +746,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    626,
 /**/
     625,
 /**/

src/vim9.h

@@ -15,6 +15,8 @@ typedef enum {
     ISN_EXEC,	    // execute Ex command line isn_arg.string
     ISN_ECHO,	    // echo isn_arg.echo.echo_count items on top of stack
     ISN_EXECUTE,    // execute Ex commands isn_arg.number items on top of stack
+    ISN_ECHOMSG,    // echo Ex commands isn_arg.number items on top of stack
+    ISN_ECHOERR,    // echo Ex commands isn_arg.number items on top of stack
 
     // get and set variables
     ISN_LOAD,	    // push local variable isn_arg.number

src/vim9compile.c

@@ -1401,14 +1401,14 @@ generate_ECHO(cctx_T *cctx, int with_white, int count)
 }
 
 /*
- * Generate an ISN_EXECUTE instruction.
+ * Generate an ISN_EXECUTE/ISN_ECHOMSG/ISN_ECHOERR instruction.
  */
     static int
-generate_EXECUTE(cctx_T *cctx, int count)
+generate_MULT_EXPR(cctx_T *cctx, isntype_T isn_type, int count)
 {
     isn_T	*isn;
 
-    if ((isn = generate_instr_drop(cctx, ISN_EXECUTE, count)) == NULL)
+    if ((isn = generate_instr_drop(cctx, isn_type, count)) == NULL)
 	return FAIL;
     isn->isn_arg.number = count;
 
@@ -1427,11 +1427,6 @@ generate_EXEC(cctx_T *cctx, char_u *line)
     return OK;
 }
 
-static char e_white_both[] =
-			N_("E1004: white space required before and after '%s'");
-static char e_white_after[] = N_("E1069: white space required after '%s'");
-static char e_no_white_before[] = N_("E1068: No white space allowed before '%s'");
-
 /*
  * Reserve space for a local variable.
  * Return the index or -1 if it failed.
@@ -4203,6 +4198,11 @@ compile_assignment(char_u *arg, exarg_T *eap, cmdidx_T cmdidx, cctx_T *cctx)
 		    goto theend;
 		}
 	    }
+	    else if (name[1] == ':' && name[2] != NUL)
+	    {
+		semsg(_("E1082: Cannot use a namespaced variable: %s"), name);
+		goto theend;
+	    }
 	}
     }
 
@@ -4211,6 +4211,11 @@ compile_assignment(char_u *arg, exarg_T *eap, cmdidx_T cmdidx, cctx_T *cctx)
 	if (is_decl && *p == ':')
 	{
 	    // parse optional type: "let var: type = expr"
+	    if (!VIM_ISWHITE(p[1]))
+	    {
+		semsg(_(e_white_after), ":");
+		goto theend;
+	    }
 	    p = skipwhite(p + 1);
 	    type = parse_type(&p, cctx->ctx_type_list);
 	    has_type = TRUE;
@@ -5768,32 +5773,12 @@ compile_throw(char_u *arg, cctx_T *cctx UNUSED)
 
 /*
  * compile "echo expr"
- */
+ * compile "echomsg expr"
-    static char_u *
+ * compile "echoerr expr"
-compile_echo(char_u *arg, int with_white, cctx_T *cctx)
-{
-    char_u	*p = arg;
-    int		count = 0;
-
-    for (;;)
-    {
-	if (compile_expr1(&p, cctx) == FAIL)
-	    return NULL;
-	++count;
-	p = skipwhite(p);
-	if (ends_excmd(*p))
-	    break;
-    }
-
-    generate_ECHO(cctx, with_white, count);
-    return p;
-}
-
-/*
  * compile "execute expr"
  */
     static char_u *
-compile_execute(char_u *arg, cctx_T *cctx)
+compile_mult_expr(char_u *arg, int cmdidx, cctx_T *cctx)
 {
     char_u	*p = arg;
     int		count = 0;
@@ -5808,8 +5793,14 @@ compile_execute(char_u *arg, cctx_T *cctx)
 	    break;
     }
 
-    generate_EXECUTE(cctx, count);
+    if (cmdidx == CMD_echo || cmdidx == CMD_echon)
-
+	generate_ECHO(cctx, cmdidx == CMD_echo, count);
+    else if (cmdidx == CMD_execute)
+	generate_MULT_EXPR(cctx, ISN_EXECUTE, count);
+    else if (cmdidx == CMD_echomsg)
+	generate_MULT_EXPR(cctx, ISN_ECHOMSG, count);
+    else
+	generate_MULT_EXPR(cctx, ISN_ECHOERR, count);
     return p;
 }
 
@@ -6184,20 +6175,16 @@ compile_def_function(ufunc_T *ufunc, int set_return_type)
 		    break;
 
 	    case CMD_echo:
-		    line = compile_echo(p, TRUE, &cctx);
-		    break;
 	    case CMD_echon:
-		    line = compile_echo(p, FALSE, &cctx);
-		    break;
 	    case CMD_execute:
-		    line = compile_execute(p, &cctx);
+	    case CMD_echomsg:
+	    case CMD_echoerr:
+		    line = compile_mult_expr(p, ea.cmdidx, &cctx);
 		    break;
 
 	    default:
 		    // Not recognized, execute with do_cmdline_cmd().
-		    // TODO:
+		    // TODO: other commands with an expression argument
-		    // CMD_echomsg
-		    // etc.
 		    generate_EXEC(&cctx, line);
 		    line = (char_u *)"";
 		    break;
@@ -6415,6 +6402,8 @@ delete_instr(isn_T *isn)
 	case ISN_DROP:
 	case ISN_ECHO:
 	case ISN_EXECUTE:
+	case ISN_ECHOMSG:
+	case ISN_ECHOERR:
 	case ISN_ENDTRY:
 	case ISN_FOR:
 	case ISN_FUNCREF:

src/vim9execute.c

@@ -668,8 +668,12 @@ call_def_function(
 		}
 		break;
 
-	    // execute :execute {string} ...
+	    // :execute {string} ...
+	    // :echomsg {string} ...
+	    // :echoerr {string} ...
 	    case ISN_EXECUTE:
+	    case ISN_ECHOMSG:
+	    case ISN_ECHOERR:
 		{
 		    int		count = iptr->isn_arg.number;
 		    garray_T	ga;
@@ -705,7 +709,30 @@ call_def_function(
 		    ectx.ec_stack.ga_len -= count;
 
 		    if (!failed && ga.ga_data != NULL)
-			do_cmdline_cmd((char_u *)ga.ga_data);
+		    {
+			if (iptr->isn_type == ISN_EXECUTE)
+			    do_cmdline_cmd((char_u *)ga.ga_data);
+			else
+			{
+			    msg_sb_eol();
+			    if (iptr->isn_type == ISN_ECHOMSG)
+			    {
+				msg_attr(ga.ga_data, echo_attr);
+				out_flush();
+			    }
+			    else
+			    {
+				int		save_did_emsg = did_emsg;
+
+				SOURCING_LNUM = iptr->isn_lnum;
+				emsg(ga.ga_data);
+				if (!force_abort)
+				    // We don't want to abort following
+				    // commands, restore did_emsg.
+				    did_emsg = save_did_emsg;
+			    }
+			}
+		    }
 		    ga_clear(&ga);
 		}
 		break;
@@ -1947,6 +1974,14 @@ ex_disassemble(exarg_T *eap)
 		smsg("%4d EXECUTE %lld", current,
 					    (long long)(iptr->isn_arg.number));
 		break;
+	    case ISN_ECHOMSG:
+		smsg("%4d ECHOMSG %lld", current,
+					    (long long)(iptr->isn_arg.number));
+		break;
+	    case ISN_ECHOERR:
+		smsg("%4d ECHOERR %lld", current,
+					    (long long)(iptr->isn_arg.number));
+		break;
 	    case ISN_LOAD:
 		if (iptr->isn_arg.number < 0)
 		    smsg("%4d LOAD arg[%lld]", current,