patch 9.0.1760: vim9 class problem with new() constructorv9.0.1760

Problem:  vim9 class problem with new() constructor
Solution: Don't allow a return type for the new() class constructor.

closes: #12863
closes: #12040

Signed-off-by: Christian Brabandt <cb@256bit.org>
Co-authored-by: Yegappan Lakshmanan <yegappan@yahoo.com>

6 files changed, 152 insertions, 8 deletions

diff --git a/runtime/doc/vim9class.txt b/runtime/doc/vim9class.txt
index 6acbca5439..a68b97c87b 100644
--- a/runtime/doc/vim9class.txt
+++ b/runtime/doc/vim9class.txt
@@ -232,6 +232,9 @@ If the class extends a parent class, the same thing happens.  In the second
 step the members of the parent class are done first.  There is no need to call
 "super()" or "new()" on the parent.
 
+When defining the new() method the return type should not be specified.  It
+always returns an object of the class.
+
 ==============================================================================
 
 3.  class members and functions			*Vim9-class-member*
diff --git a/src/errors.h b/src/errors.h
index 67012e7046..4b58cbbda8 100644
--- a/src/errors.h
+++ b/src/errors.h
@@ -3478,6 +3478,8 @@ EXTERN char e_incomplete_type[]
 #endif
 EXTERN char e_warning_pointer_block_corrupted[]
 	INIT(= N_("E1364: Warning: Pointer block corrupted"));
+EXTERN char e_cannot_use_a_return_type_with_new[]
+	INIT(= N_("E1365: Cannot use a return type with the \"new\" function"));
 EXTERN char e_cannot_mix_positional_and_non_positional_str[]
 	INIT(= N_("E1400: Cannot mix positional and non-positional arguments: %s"));
 EXTERN char e_fmt_arg_nr_unused_str[]
@@ -3495,4 +3497,4 @@ EXTERN char e_member_str_type_mismatch_expected_str_but_got_str[]
 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
+// E1366 - E1399 unused
diff --git a/src/testdir/test_vim9_class.vim b/src/testdir/test_vim9_class.vim
index bc90e42597..dbd4c3bcbe 100644
--- a/src/testdir/test_vim9_class.vim
+++ b/src/testdir/test_vim9_class.vim
@@ -1577,6 +1577,16 @@ def Test_class_implements_interface()
   END
   v9.CheckScriptFailure(lines, 'E1349:')
 
+  # implements should be followed by a white space
+  lines =<< trim END
+    vim9script
+    interface A
+    endinterface
+    class B implements A;
+    endclass
+  END
+  v9.CheckScriptFailure(lines, 'E1315:')
+
   lines =<< trim END
       vim9script
 
@@ -2515,4 +2525,104 @@ def Test_stack_expansion_with_methods()
   END
   v9.CheckScriptSuccess(lines)
 enddef
+
+" Test the return type of the new() constructor
+def Test_new_return_type()
+  # new() uses the default return type and there is no return statement
+  var lines =<< trim END
+    vim9script
+
+    class C
+      this._bufnr: number
+
+      def new(this._bufnr)
+        if !bufexists(this._bufnr)
+          this._bufnr = -1
+        endif
+      enddef
+    endclass
+
+    var c = C.new(12345)
+    assert_equal('object<C>', typename(c))
+
+    var v1: C
+    v1 = C.new(12345)
+    assert_equal('object<C>', typename(v1))
+
+    def F()
+      var v2: C
+      v2 = C.new(12345)
+      assert_equal('object<C>', typename(v2))
+    enddef
+    F()
+  END
+  v9.CheckScriptSuccess(lines)
+
+  # new() uses the default return type and an empty 'return' statement
+  lines =<< trim END
+    vim9script
+
+    class C
+      this._bufnr: number
+
+      def new(this._bufnr)
+        if !bufexists(this._bufnr)
+          this._bufnr = -1
+          return
+        endif
+      enddef
+    endclass
+
+    var c = C.new(12345)
+    assert_equal('object<C>', typename(c))
+
+    var v1: C
+    v1 = C.new(12345)
+    assert_equal('object<C>', typename(v1))
+
+    def F()
+      var v2: C
+      v2 = C.new(12345)
+      assert_equal('object<C>', typename(v2))
+    enddef
+    F()
+  END
+  v9.CheckScriptSuccess(lines)
+
+  # new() uses "any" return type and returns "this"
+  lines =<< trim END
+    vim9script
+
+    class C
+      this._bufnr: number
+
+      def new(this._bufnr): any
+        if !bufexists(this._bufnr)
+          this._bufnr = -1
+          return this
+        endif
+      enddef
+    endclass
+  END
+  v9.CheckScriptFailure(lines, 'E1365:')
+
+  # new() uses 'Dict' return type and returns a Dict
+  lines =<< trim END
+    vim9script
+
+    class C
+      this._state: dict<any>
+
+      def new(): dict<any>
+        this._state = {}
+        return this._state
+      enddef
+    endclass
+
+    var c = C.new()
+    assert_equal('object<C>', typename(c))
+  END
+  v9.CheckScriptFailure(lines, 'E1365:')
+enddef
+
 " vim: ts=8 sw=2 sts=2 expandtab tw=80 fdm=marker
diff --git a/src/version.c b/src/version.c
index 2da6a2074e..93192c3236 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 */
 /**/
+    1760,
+/**/
     1759,
 /**/
     1758,
diff --git a/src/vim9class.c b/src/vim9class.c
index 9e5bb56bf1..e4130459b2 100644
--- a/src/vim9class.c
+++ b/src/vim9class.c
@@ -687,7 +687,7 @@ add_class_members(class_T *cl, exarg_T *eap)
 }
 
 /*
- * Add a default constructor to the class "cl".
+ * Add a default constructor method (new()) to the class "cl".
  */
     static void
 add_default_constructor(
@@ -1150,6 +1150,18 @@ early_ret:
 		    func_clear_free(uf, FALSE);
 		    break;
 		}
+		if (is_new)
+		{
+		    // A return type should not be specified for the new()
+		    // constructor method.
+		    if (uf->uf_ret_type->tt_type != VAR_VOID)
+		    {
+			emsg(_(e_cannot_use_a_return_type_with_new));
+			success = FALSE;
+			func_clear_free(uf, FALSE);
+			break;
+		    }
+		}
 		garray_T *fgap = has_static || is_new
 					       ? &classfunctions : &objmethods;
 		// Check the name isn't used already.
@@ -1303,15 +1315,22 @@ early_ret:
 	if (is_class && cl->class_class_member_count > 0)
 	    add_class_members(cl, eap);
 
-	int have_new = FALSE;
+	int	have_new = FALSE;
+	ufunc_T	*class_func = NULL;
 	for (int i = 0; i < classfunctions.ga_len; ++i)
-	    if (STRCMP(((ufunc_T **)classfunctions.ga_data)[i]->uf_name,
-								   "new") == 0)
+	{
+	    class_func = ((ufunc_T **)classfunctions.ga_data)[i];
+	    if (STRCMP(class_func->uf_name, "new") == 0)
 	    {
 		have_new = TRUE;
 		break;
 	    }
-	if (is_class && !is_abstract && !have_new)
+	}
+
+	if (have_new)
+	    // The return type of new() is an object of class "cl"
+	    class_func->uf_ret_type->tt_class = cl;
+	else if (is_class && !is_abstract && !have_new)
 	    // No new() method was defined, add the default constructor.
 	    add_default_constructor(cl, &classfunctions, &type_list);
 
diff --git a/src/vim9cmds.c b/src/vim9cmds.c
index 29ed054859..3f44b88733 100644
--- a/src/vim9cmds.c
+++ b/src/vim9cmds.c
@@ -2617,8 +2617,16 @@ compile_return(char_u *arg, int check_return_type, int legacy, cctx_T *cctx)
 	    return NULL;
 	}
 
-	// No argument, return zero.
-	generate_PUSHNR(cctx, 0);
+	if (cctx->ctx_ufunc->uf_flags & FC_NEW)
+	{
+	    // For a class new() constructor, return an object of the class.
+	    generate_instr(cctx, ISN_RETURN_OBJECT);
+	    cctx->ctx_ufunc->uf_ret_type =
+		&cctx->ctx_ufunc->uf_class->class_object_type;
+	}
+	else
+	    // No argument, return zero.
+	    generate_PUSHNR(cctx, 0);
     }
 
     // may need ENDLOOP when inside a :for or :while loop