patch 9.0.1741: No type checking in interfacesv9.0.1741

Problem: No type checking in interfaces
Solution: Implement member type check in vim9 interfaces

Most of the code is a small refactoring to allow the use of a where_T
for signaling the type mismatch, the type checking itself is pretty
simple.

Improve where_T error reports

Let the caller explicitly define the kind of location it's referring to
and free the WT_ARGUMENT enum from its catch-all role.

Implement type checking for interface methods

Follows closely the logic used for type-checking the members.

closes: #12844

Signed-off-by: Christian Brabandt <cb@256bit.org>
Co-authored-by: LemonBoy <thatlemon@gmail.com>

12 files changed, 176 insertions, 53 deletions

diff --git a/src/errors.h b/src/errors.h
index 1c1b146c9d..67012e7046 100644
--- a/src/errors.h
+++ b/src/errors.h
@@ -3490,5 +3490,9 @@ EXTERN char e_positional_arg_num_type_inconsistent_str_str[]
 	INIT(= N_("E1404: Positional argument %d type used inconsistently: %s/%s"));
 EXTERN char e_invalid_format_specifier_str[]
 	INIT(= N_("E1405: Invalid format specifier: %s"));
+EXTERN char e_member_str_type_mismatch_expected_str_but_got_str[]
+	INIT(= N_("E1406: Member \"%s\": type mismatch, expected %s but got %s"));
+EXTERN char e_method_str_type_mismatch_expected_str_but_got_str[]
+	INIT(= N_("E1407: Member \"%s\": type mismatch, expected %s but got %s"));
 
 // E1365 - E1399 unused
diff --git a/src/eval.c b/src/eval.c
index cfcec9bac0..ad02c2c965 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -3859,7 +3859,6 @@ eval8(
 		{
 		    where_T where = WHERE_INIT;
 
-		    where.wt_variable = TRUE;
 		    res = check_type(want_type, actual, TRUE, where);
 		}
 	    }
diff --git a/src/evalfunc.c b/src/evalfunc.c
index c0710da119..8561e94d22 100644
--- a/src/evalfunc.c
+++ b/src/evalfunc.c
@@ -647,6 +647,7 @@ check_map_filter_arg2(type_T *type, argcontext_T *context, int is_map)
 	    t_func_exp.tt_argcount = -1;
 
 	where.wt_index = 2;
+	where.wt_kind = WT_ARGUMENT;
 	return check_type(&t_func_exp, type, TRUE, where);
     }
     return OK;
@@ -714,6 +715,7 @@ arg_sort_how(type_T *type, type_T *decl_type UNUSED, argcontext_T *context)
 	    if (type->tt_argcount == -1)
 		t_func_exp.tt_argcount = -1;
 	    where.wt_index = 2;
+	    where.wt_kind = WT_ARGUMENT;
 	    return check_type(&t_func_exp, type, TRUE, where);
 	}
 
diff --git a/src/evalvars.c b/src/evalvars.c
index 1033004d79..cb31966bd2 100644
--- a/src/evalvars.c
+++ b/src/evalvars.c
@@ -3839,8 +3839,11 @@ set_var_const(
 		if (sv != NULL)
 		{
 		    // check the type and adjust to bool if needed
-		    where.wt_index = var_idx;
-		    where.wt_variable = TRUE;
+		    if (var_idx > 0)
+		    {
+			where.wt_index = var_idx;
+			where.wt_kind = WT_VARIABLE;
+		    }
 		    if (check_script_var_type(sv, tv, name, where) == FAIL)
 			goto failed;
 		    if (type == NULL)
diff --git a/src/structs.h b/src/structs.h
index d8c6aef956..eefc8f2186 100644
--- a/src/structs.h
+++ b/src/structs.h
@@ -4764,14 +4764,22 @@ typedef enum {
     MAGIC_ALL = 4		// "\v" very magic
 } magic_T;
 
+typedef enum {
+    WT_UNKNOWN = 0,	// Unknown or unspecified location
+    WT_ARGUMENT,
+    WT_VARIABLE,
+    WT_MEMBER,
+    WT_METHOD,
+} wherekind_T;
+
 // Struct used to pass to error messages about where the error happened.
 typedef struct {
-    char    *wt_func_name;  // function name or NULL
-    char    wt_index;	    // argument or variable index, 0 means unknown
-    char    wt_variable;    // "variable" when TRUE, "argument" otherwise
+    char	*wt_func_name;  // function name or NULL
+    char	wt_index;	// argument or variable index, 0 means unknown
+    wherekind_T	wt_kind;	// "variable" when TRUE, "argument" otherwise
 } where_T;
 
-#define WHERE_INIT {NULL, 0, 0}
+#define WHERE_INIT {NULL, 0, WT_UNKNOWN}
 
 // Struct passed to get_v_event() and restore_v_event().
 typedef struct {
diff --git a/src/testdir/test_vim9_class.vim b/src/testdir/test_vim9_class.vim
index 684bf6d1f6..a79ccc8f53 100644
--- a/src/testdir/test_vim9_class.vim
+++ b/src/testdir/test_vim9_class.vim
@@ -1576,6 +1576,61 @@ def Test_class_implements_interface()
     endclass
   END
   v9.CheckScriptFailure(lines, 'E1349:')
+
+  lines =<< trim END
+      vim9script
+
+      interface One
+        static matching: bool
+        static as_any: any
+        static not_matching: number
+      endinterface
+      class Two implements One
+        static not_matching: string
+        static as_any: string
+        static matching: bool
+      endclass
+  END
+  v9.CheckScriptFailure(lines, 'E1406: Member "not_matching": type mismatch, expected number but got string')
+
+  lines =<< trim END
+      vim9script
+
+      interface One
+        def IsEven(nr: number): bool
+      endinterface
+      class Two implements One
+        def IsEven(nr: number): string
+        enddef
+      endclass
+  END
+  v9.CheckScriptFailure(lines, 'E1407: Member "IsEven": type mismatch, expected func(number): bool but got func(number): string')
+
+  lines =<< trim END
+      vim9script
+
+      interface One
+        def IsEven(nr: number): bool
+      endinterface
+      class Two implements One
+        def IsEven(nr: bool): bool
+        enddef
+      endclass
+  END
+  v9.CheckScriptFailure(lines, 'E1407: Member "IsEven": type mismatch, expected func(number): bool but got func(bool): bool')
+
+  lines =<< trim END
+      vim9script
+
+      interface One
+        def IsEven(nr: number): bool
+      endinterface
+      class Two implements One
+        def IsEven(nr: number, ...extra: list<number>): bool
+        enddef
+      endclass
+  END
+  v9.CheckScriptFailure(lines, 'E1407: Member "IsEven": type mismatch, expected func(number): bool but got func(number, ...list<number>): bool')
 enddef
 
 def Test_call_interface_method()
diff --git a/src/version.c b/src/version.c
index fbf4c68691..ff74ff4a9c 100644
--- a/src/version.c
+++ b/src/version.c
@@ -696,6 +696,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    1741,
+/**/
     1740,
 /**/
     1739,
diff --git a/src/vim9class.c b/src/vim9class.c
index e310dec336..3822753a4e 100644
--- a/src/vim9class.c
+++ b/src/vim9class.c
@@ -795,12 +795,20 @@ early_ret:
 		    int cl_i;
 		    for (cl_i = 0; cl_i < cl_count; ++cl_i)
 		    {
-			ocmember_T *m = &cl_ms[cl_i];
-			if (STRCMP(if_ms[if_i].ocm_name, m->ocm_name) == 0)
-			{
-			    // TODO: check type
-			    break;
-			}
+			ocmember_T	*m = &cl_ms[cl_i];
+			where_T		where = WHERE_INIT;
+
+			if (STRCMP(if_ms[if_i].ocm_name, m->ocm_name) != 0)
+			    continue;
+
+			// Ensure the type is matching.
+			where.wt_func_name = m->ocm_name;
+			where.wt_kind = WT_MEMBER;
+			if (check_type_maybe(if_ms[if_i].ocm_type, m->ocm_type, TRUE,
+				    where) != OK)
+			    success = FALSE;
+
+			break;
 		    }
 		    if (cl_i == cl_count)
 		    {
@@ -838,7 +846,14 @@ early_ret:
 			char_u *cl_name = cl_fp[cl_i]->uf_name;
 			if (STRCMP(if_name, cl_name) == 0)
 			{
-			    // TODO: check return and argument types
+			    where_T where = WHERE_INIT;
+
+			    // Ensure the type is matching.
+			    where.wt_func_name = (char *)if_name;
+			    where.wt_kind = WT_METHOD;
+			    if (check_type_maybe(if_fp[if_i]->uf_func_type,
+					cl_fp[cl_i]->uf_func_type, TRUE, where) != OK)
+				success = FALSE;
 			    break;
 			}
 		    }
@@ -1139,6 +1154,7 @@ early_ret:
 			{
 			    where_T where = WHERE_INIT;
 			    where.wt_func_name = (char *)pname;
+			    where.wt_kind = WT_METHOD;
 			    (void)check_type(pf->uf_func_type, cf->uf_func_type,
 								  TRUE, where);
 			}
diff --git a/src/vim9cmds.c b/src/vim9cmds.c
index c3c900a5be..29ed054859 100644
--- a/src/vim9cmds.c
+++ b/src/vim9cmds.c
@@ -1045,8 +1045,11 @@ compile_for(char_u *arg_start, cctx_T *cctx)
 		}
 
 		// Reserve a variable to store "var".
-		where.wt_index = var_list ? idx + 1 : 0;
-		where.wt_variable = TRUE;
+		if (var_list)
+		{
+		    where.wt_index = idx + 1;
+		    where.wt_kind = WT_VARIABLE;
+		}
 		if (lhs_type == &t_any)
 		    lhs_type = item_type;
 		else if (item_type != &t_unknown
diff --git a/src/vim9compile.c b/src/vim9compile.c
index e6663570f0..028b0ca152 100644
--- a/src/vim9compile.c
+++ b/src/vim9compile.c
@@ -496,7 +496,7 @@ need_type_where(
     if (!actual_is_const && ret == MAYBE && use_typecheck(actual, expected))
     {
 	generate_TYPECHECK(cctx, expected, number_ok, offset,
-					    where.wt_variable, where.wt_index);
+		where.wt_kind == WT_VARIABLE, where.wt_index);
 	return OK;
     }
 
@@ -518,7 +518,11 @@ need_type(
 {
     where_T where = WHERE_INIT;
 
-    where.wt_index = arg_idx;
+    if (arg_idx > 0)
+    {
+	where.wt_index = arg_idx;
+	where.wt_kind = WT_ARGUMENT;
+    }
     return need_type_where(actual, expected, number_ok, offset, where,
 						cctx, silent, actual_is_const);
 }
@@ -2636,8 +2640,11 @@ compile_assignment(
 			// Without operator check type here, otherwise below.
 			// Use the line number of the assignment.
 			SOURCING_LNUM = start_lnum;
-			where.wt_index = var_count > 0 ? var_idx + 1 : 0;
-			where.wt_variable = var_count > 0;
+			if (var_count > 0)
+			{
+			    where.wt_index = var_idx + 1;
+			    where.wt_kind = WT_VARIABLE;
+			}
 			// If assigning to a list or dict member, use the
 			// member type.  Not for "list[:] =".
 			if (lhs.lhs_has_index
@@ -3193,6 +3200,7 @@ compile_def_function(
 	    // specified type.
 	    val_type = get_type_on_stack(&cctx, 0);
 	    where.wt_index = arg_idx + 1;
+	    where.wt_kind = WT_ARGUMENT;
 	    if (ufunc->uf_arg_types[arg_idx] == &t_unknown)
 	    {
 		did_set_arg_type = TRUE;
diff --git a/src/vim9execute.c b/src/vim9execute.c
index eb663a1096..6c208786a3 100644
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -5272,7 +5272,7 @@ exec_instructions(ectx_T *ectx)
 		    // Useful when used in unpack assignment.  Reset at
 		    // ISN_DROP.
 		    ectx->ec_where.wt_index = gi->gi_index + 1;
-		    ectx->ec_where.wt_variable = TRUE;
+		    ectx->ec_where.wt_kind = WT_VARIABLE;
 		}
 		break;
 
@@ -5442,20 +5442,20 @@ exec_instructions(ectx_T *ectx)
 	    case ISN_CHECKTYPE:
 		{
 		    checktype_T *ct = &iptr->isn_arg.type;
-		    int		save_wt_variable = ectx->ec_where.wt_variable;
 		    int		r;
+		    where_T	where = WHERE_INIT;
 
 		    tv = STACK_TV_BOT((int)ct->ct_off);
 		    SOURCING_LNUM = iptr->isn_lnum;
-		    if (!ectx->ec_where.wt_variable)
-			ectx->ec_where.wt_index = ct->ct_arg_idx;
-		    ectx->ec_where.wt_variable = ct->ct_is_var;
-		    r = check_typval_type(ct->ct_type, tv, ectx->ec_where);
-		    ectx->ec_where.wt_variable = save_wt_variable;
+		    if (ct->ct_arg_idx > 0)
+		    {
+			where.wt_index = ct->ct_arg_idx;
+			where.wt_kind = ct->ct_is_var ? WT_VARIABLE : WT_ARGUMENT;
+			where.wt_func_name = ectx->ec_where.wt_func_name;
+		    }
+		    r = check_typval_type(ct->ct_type, tv, where);
 		    if (r == FAIL)
 			goto on_error;
-		    if (!ectx->ec_where.wt_variable)
-			ectx->ec_where.wt_index = 0;
 
 		    // number 0 is FALSE, number 1 is TRUE
 		    if (tv->v_type == VAR_NUMBER
@@ -5736,8 +5736,7 @@ exec_instructions(ectx_T *ectx)
 	    case ISN_DROP:
 		--ectx->ec_stack.ga_len;
 		clear_tv(STACK_TV_BOT(0));
-		ectx->ec_where.wt_index = 0;
-		ectx->ec_where.wt_variable = FALSE;
+		ectx->ec_where = (where_T)WHERE_INIT;
 		break;
 	}
 	continue;
@@ -6170,8 +6169,7 @@ call_def_function(
     emsg_silent_def = emsg_silent;
     did_emsg_def = 0;
 
-    ectx.ec_where.wt_index = 0;
-    ectx.ec_where.wt_variable = FALSE;
+    ectx.ec_where = (where_T)WHERE_INIT;
 
     /*
      * Execute the instructions until done.
diff --git a/src/vim9type.c b/src/vim9type.c
index 0dd74ee184..1363a1c3e7 100644
--- a/src/vim9type.c
+++ b/src/vim9type.c
@@ -678,8 +678,12 @@ check_typval_arg_type(
 {
     where_T	where = WHERE_INIT;
 
-    where.wt_index = arg_idx;
-    where.wt_func_name = func_name;
+    if (arg_idx > 0)
+    {
+	where.wt_index = arg_idx;
+	where.wt_kind = WT_ARGUMENT;
+	where.wt_func_name = func_name;
+    }
     return check_typval_type(expected, actual_tv, where);
 }
 
@@ -733,7 +737,11 @@ arg_type_mismatch(type_T *expected, type_T *actual, int arg_idx)
 {
     where_T	where = WHERE_INIT;
 
-    where.wt_index = arg_idx;
+    if (arg_idx > 0)
+    {
+	where.wt_index = arg_idx;
+	where.wt_kind = WT_ARGUMENT;
+    }
     type_mismatch_where(expected, actual, where);
 }
 
@@ -744,25 +752,42 @@ type_mismatch_where(type_T *expected, type_T *actual, where_T where)
     char *typename1 = type_name(expected, &tofree1);
     char *typename2 = type_name(actual, &tofree2);
 
-    if (where.wt_index > 0)
+    switch (where.wt_kind)
     {
-	if (where.wt_func_name == NULL)
-	    semsg(_(where.wt_variable
-			 ? e_variable_nr_type_mismatch_expected_str_but_got_str
-		       : e_argument_nr_type_mismatch_expected_str_but_got_str),
-					 where.wt_index, typename1, typename2);
-	else
-	    semsg(_(where.wt_variable
-		  ? e_variable_nr_type_mismatch_expected_str_but_got_str_in_str
-		: e_argument_nr_type_mismatch_expected_str_but_got_str_in_str),
-		     where.wt_index, typename1, typename2, where.wt_func_name);
+	case WT_MEMBER:
+	    semsg(_(e_member_str_type_mismatch_expected_str_but_got_str),
+		    where.wt_func_name, typename1, typename2);
+	    break;
+	case WT_METHOD:
+	    semsg(_(e_method_str_type_mismatch_expected_str_but_got_str),
+		    where.wt_func_name, typename1, typename2);
+	    break;
+	case WT_VARIABLE:
+	    if (where.wt_func_name == NULL)
+		semsg(_(e_variable_nr_type_mismatch_expected_str_but_got_str),
+			where.wt_index, typename1, typename2);
+	    else
+		semsg(_(e_variable_nr_type_mismatch_expected_str_but_got_str_in_str),
+			where.wt_index, typename1, typename2, where.wt_func_name);
+	    break;
+	case WT_ARGUMENT:
+	    if (where.wt_func_name == NULL)
+		semsg(_(e_argument_nr_type_mismatch_expected_str_but_got_str),
+			where.wt_index, typename1, typename2);
+	    else
+		semsg(_(e_argument_nr_type_mismatch_expected_str_but_got_str_in_str),
+			where.wt_index, typename1, typename2, where.wt_func_name);
+	    break;
+	case WT_UNKNOWN:
+	    if (where.wt_func_name == NULL)
+		semsg(_(e_type_mismatch_expected_str_but_got_str),
+			typename1, typename2);
+	    else
+		semsg(_(e_type_mismatch_expected_str_but_got_str_in_str),
+			typename1, typename2, where.wt_func_name);
+	    break;
     }
-    else if (where.wt_func_name == NULL)
-	semsg(_(e_type_mismatch_expected_str_but_got_str),
-							 typename1, typename2);
-    else
-	semsg(_(e_type_mismatch_expected_str_but_got_str_in_str),
-				     typename1, typename2, where.wt_func_name);
+
     vim_free(tofree1);
     vim_free(tofree2);
 }