patch 8.2.2652: Vim9: can use command modifier without an effect

Problem:    Vim9: can use command modifier without an effect.
Solution:   Give an error for a misplaced command modifier.  Fix error message
            number.

Author: brammool

src/ex_docmd.c

@@ -2969,6 +2969,33 @@ parse_command_modifiers(
     return OK;
 }
 
+/*
+ * Return TRUE if "cmod" has anything set.
+ */
+    int
+has_cmdmod(cmdmod_T *cmod)
+{
+    return cmod->cmod_flags != 0
+	    || cmod->cmod_split != 0
+	    || cmod->cmod_verbose != 0
+	    || cmod->cmod_tab != 0
+	    || cmod->cmod_filter_regmatch.regprog != NULL;
+}
+
+/*
+ * If Vim9 script and "cmdmod" has anything set give an error and return TRUE.
+ */
+    int
+cmdmod_error(void)
+{
+    if (in_vim9script() && has_cmdmod(&cmdmod))
+    {
+	emsg(_(e_misplaced_command_modifier));
+	return TRUE;
+    }
+    return FALSE;
+}
+
 /*
  * Apply the command modifiers.  Saves current state in "cmdmod", call
  * undo_cmdmod() later.

src/ex_eval.c

@@ -1011,6 +1011,8 @@ ex_endif(exarg_T *eap)
 {
     cstack_T	*cstack = eap->cstack;
 
+    if (cmdmod_error())
+	return;
     did_endif = TRUE;
     if (cstack->cs_idx < 0
 	    || (cstack->cs_flags[cstack->cs_idx]
@@ -1314,6 +1316,9 @@ ex_endwhile(exarg_T *eap)
     int		csf;
     int		fl;
 
+    if (cmdmod_error())
+	return;
+
     if (eap->cmdidx == CMD_endwhile)
     {
 	err = e_while;
@@ -1539,6 +1544,9 @@ ex_try(exarg_T *eap)
     int		skip;
     cstack_T	*cstack = eap->cstack;
 
+    if (cmdmod_error())
+	return;
+
     if (cstack->cs_idx == CSTACK_LEN - 1)
 	eap->errmsg = _("E601: :try nesting too deep");
     else
@@ -1617,6 +1625,9 @@ ex_catch(exarg_T *eap)
     cstack_T	*cstack = eap->cstack;
     char_u	*pat;
 
+    if (cmdmod_error())
+	return;
+
     if (cstack->cs_trylevel <= 0 || cstack->cs_idx < 0)
     {
 	eap->errmsg = _(e_catch);
@@ -1777,6 +1788,9 @@ ex_finally(exarg_T *eap)
     int		pending = CSTP_NONE;
     cstack_T	*cstack = eap->cstack;
 
+    if (cmdmod_error())
+	return;
+
     if (cstack->cs_trylevel <= 0 || cstack->cs_idx < 0)
 	eap->errmsg = _(e_finally);
     else
@@ -1906,6 +1920,9 @@ ex_endtry(exarg_T *eap)
     void	*rettv = NULL;
     cstack_T	*cstack = eap->cstack;
 
+    if (cmdmod_error())
+	return;
+
     if (cstack->cs_trylevel <= 0 || cstack->cs_idx < 0)
 	eap->errmsg = _(e_no_endtry);
     else

src/proto/ex_docmd.pro

@@ -8,6 +8,8 @@ void *getline_cookie(char_u *(*fgetline)(int, void *, int, getline_opt_T), void
 char_u *getline_peek(char_u *(*fgetline)(int, void *, int, getline_opt_T), void *cookie);
 char *ex_errmsg(char *msg, char_u *arg);
 int parse_command_modifiers(exarg_T *eap, char **errormsg, cmdmod_T *cmod, int skip_only);
+int has_cmdmod(cmdmod_T *cmod);
+int cmdmod_error(void);
 void apply_cmdmod(cmdmod_T *cmod);
 void undo_cmdmod(cmdmod_T *cmod);
 int parse_cmd_address(exarg_T *eap, char **errormsg, int silent);

src/testdir/test_vim9_builtin.vim

@@ -247,7 +247,7 @@ enddef
 def Test_exepath()
   CheckDefExecFailure(['echo exepath(true)'], 'E1174:')
   CheckDefExecFailure(['echo exepath(v:null)'], 'E1174:')
-  CheckDefExecFailure(['echo exepath("")'], 'E1142:')
+  CheckDefExecFailure(['echo exepath("")'], 'E1175:')
 enddef
 
 def Test_expand()
@@ -406,13 +406,13 @@ enddef
 def Test_finddir()
   CheckDefExecFailure(['echo finddir(true)'], 'E1174:')
   CheckDefExecFailure(['echo finddir(v:null)'], 'E1174:')
-  CheckDefExecFailure(['echo finddir("")'], 'E1142:')
+  CheckDefExecFailure(['echo finddir("")'], 'E1175:')
 enddef
 
 def Test_findfile()
   CheckDefExecFailure(['echo findfile(true)'], 'E1174:')
   CheckDefExecFailure(['echo findfile(v:null)'], 'E1174:')
-  CheckDefExecFailure(['echo findfile("")'], 'E1142:')
+  CheckDefExecFailure(['echo findfile("")'], 'E1175:')
 enddef
 
 def Test_flattennew()

src/testdir/test_vim9_cmd.vim

@@ -797,6 +797,55 @@ def Test_silent_pattern()
   bwipe!
 enddef
 
+def Test_useless_command_modifier()
+  g:maybe = true
+  var lines =<< trim END
+      if g:maybe
+      silent endif
+  END
+  CheckDefAndScriptFailure(lines, 'E1176:', 2)
+
+  lines =<< trim END
+      for i in [0]
+      silent endfor
+  END
+  CheckDefAndScriptFailure(lines, 'E1176:', 2)
+
+  lines =<< trim END
+      while g:maybe
+      silent endwhile
+  END
+  CheckDefAndScriptFailure(lines, 'E1176:', 2)
+
+  lines =<< trim END
+      silent try
+      finally
+      endtry
+  END
+  CheckDefAndScriptFailure(lines, 'E1176:', 1)
+
+  lines =<< trim END
+      try
+      silent catch
+      endtry
+  END
+  CheckDefAndScriptFailure(lines, 'E1176:', 2)
+
+  lines =<< trim END
+      try
+      silent finally
+      endtry
+  END
+  CheckDefAndScriptFailure(lines, 'E1176:', 2)
+
+  lines =<< trim END
+      try
+      finally
+      silent endtry
+  END
+  CheckDefAndScriptFailure(lines, 'E1176:', 3)
+enddef
+
 def Test_eval_command()
   var from = 3
   var to = 5

src/testdir/test_vim9_disassemble.vim

@@ -1903,7 +1903,7 @@ enddef
 def s:SilentIf()
   silent if 4 == g:five
   silent elseif 4 == g:five
-  silent endif
+  endif
 enddef
 
 def Test_silent_if()
@@ -1924,14 +1924,14 @@ def Test_silent_if()
         '\d\+ COMPAREANY ==\_s*' ..
         '\d\+ CMDMOD_REV\_s*' ..
         '\d\+ JUMP_IF_FALSE -> \d\+\_s*' ..
-        'silent endif\_s*' ..
+        'endif\_s*' ..
         '\d\+ RETURN 0',
         res)
 enddef
 
 def s:SilentFor()
   silent for i in [0]
-  silent endfor
+  endfor
 enddef
 
 def Test_silent_for()
@@ -1945,7 +1945,7 @@ def Test_silent_for()
         '\d CMDMOD_REV\_s*' ..
         '5 FOR $0 -> 8\_s*' ..
         '\d STORE $1\_s*' ..
-        'silent endfor\_s*' ..
+        'endfor\_s*' ..
         '\d JUMP -> 5\_s*' ..
         '8 DROP\_s*' ..
         '\d RETURN 0\_s*',
@@ -1954,7 +1954,7 @@ enddef
 
 def s:SilentWhile()
   silent while g:not
-  silent endwhile
+  endwhile
 enddef
 
 def Test_silent_while()
@@ -1967,7 +1967,7 @@ def Test_silent_while()
         '\d CMDMOD_REV\_s*' ..
         '\d JUMP_IF_FALSE -> 6\_s*' ..
 
-        'silent endwhile\_s*' ..
+        'endwhile\_s*' ..
         '\d JUMP -> 0\_s*' ..
         '6 RETURN 0\_s*',
          res)

src/version.c

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

src/vim9compile.c

@@ -2142,11 +2142,7 @@ generate_cmdmods(cctx_T *cctx, cmdmod_T *cmod)
 {
     isn_T	*isn;
 
-    if (cmod->cmod_flags != 0
-	    || cmod->cmod_split != 0
-	    || cmod->cmod_verbose != 0
-	    || cmod->cmod_tab != 0
-	    || cmod->cmod_filter_regmatch.regprog != NULL)
+    if (has_cmdmod(cmod))
     {
 	cctx->ctx_has_cmdmod = TRUE;
 
@@ -2172,22 +2168,19 @@ generate_undo_cmdmods(cctx_T *cctx)
     return OK;
 }
 
-/*
- * If an ISN_CMDMOD was just generated drop it.
- */
-    static void
-drop_cmdmod(cctx_T *cctx)
+    static int
+misplaced_cmdmod(cctx_T *cctx)
 {
     garray_T	*instr = &cctx->ctx_instr;
 
-    // Drop any CMDMOD instruction
     if (cctx->ctx_has_cmdmod
 	    && ((isn_T *)instr->ga_data)[instr->ga_len - 1].isn_type
 								 == ISN_CMDMOD)
     {
-	--instr->ga_len;
-	cctx->ctx_has_cmdmod = FALSE;
+	emsg(_(e_misplaced_command_modifier));
+	return TRUE;
     }
+    return FALSE;
 }
 
 /*
@@ -7147,7 +7140,9 @@ compile_endif(char_u *arg, cctx_T *cctx)
     garray_T	*instr = &cctx->ctx_instr;
     isn_T	*isn;
 
-    drop_cmdmod(cctx);
+    if (misplaced_cmdmod(cctx))
+	return NULL;
+
     if (scope == NULL || scope->se_type != IF_SCOPE)
     {
 	emsg(_(e_endif_without_if));
@@ -7393,7 +7388,8 @@ compile_endfor(char_u *arg, cctx_T *cctx)
     forscope_T	*forscope;
     isn_T	*isn;
 
-    drop_cmdmod(cctx);
+    if (misplaced_cmdmod(cctx))
+	return NULL;
 
     if (scope == NULL || scope->se_type != FOR_SCOPE)
     {
@@ -7479,7 +7475,8 @@ compile_endwhile(char_u *arg, cctx_T *cctx)
     scope_T	*scope = cctx->ctx_scope;
     garray_T	*instr = &cctx->ctx_instr;
 
-    drop_cmdmod(cctx);
+    if (misplaced_cmdmod(cctx))
+	return NULL;
     if (scope == NULL || scope->se_type != WHILE_SCOPE)
     {
 	emsg(_(e_while));
@@ -7644,6 +7641,9 @@ compile_try(char_u *arg, cctx_T *cctx)
     scope_T	*try_scope;
     scope_T	*scope;
 
+    if (misplaced_cmdmod(cctx))
+	return NULL;
+
     // scope that holds the jumps that go to catch/finally/endtry
     try_scope = new_scope(cctx, TRY_SCOPE);
     if (try_scope == NULL)
@@ -7684,6 +7684,9 @@ compile_catch(char_u *arg, cctx_T *cctx UNUSED)
     char_u	*p;
     isn_T	*isn;
 
+    if (misplaced_cmdmod(cctx))
+	return NULL;
+
     // end block scope from :try or :catch
     if (scope != NULL && scope->se_type == BLOCK_SCOPE)
 	compile_endblock(cctx);
@@ -7796,6 +7799,9 @@ compile_finally(char_u *arg, cctx_T *cctx)
     isn_T	*isn;
     int		this_instr;
 
+    if (misplaced_cmdmod(cctx))
+	return NULL;
+
     // end block scope from :try or :catch
     if (scope != NULL && scope->se_type == BLOCK_SCOPE)
 	compile_endblock(cctx);
@@ -7854,6 +7860,9 @@ compile_endtry(char_u *arg, cctx_T *cctx)
     garray_T	*instr = &cctx->ctx_instr;
     isn_T	*try_isn;
 
+    if (misplaced_cmdmod(cctx))
+	return NULL;
+
     // end block scope from :catch or :finally
     if (scope != NULL && scope->se_type == BLOCK_SCOPE)
 	compile_endblock(cctx);