patch 9.0.1760: vim9 class problem with new() constructor

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>

diff --git a/runtime/doc/vim9class.txt b/runtime/doc/vim9class.txt
index 6acbca54393a5f65331b5ee393d2b5cf5636196a..a68b97c87b991ee152ee5ae4a74a2dc2d76edf46 100644 (file)
--- 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 67012e7046ce4ed415e643b4a42887d4bcc790e4..4b58cbbda84593c6f4fa8680fe022fbdf37ad18a 100644 (file)
--- 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 bc90e42597bc7b9ed178091b186297253fdc5dad..dbd4c3bcbec2f4e76fde7fce0b0b4a9775958f92 100644 (file)
--- 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 2da6a2074e8a765c7943ba6d99ef94014f186db1..93192c3236477bb13ea1f67b583e3a99a62edd84 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -695,6 +695,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1760,
 /**/
     1759,
 /**/

diff --git a/src/vim9class.c b/src/vim9class.c
index 9e5bb56bf1db8cdbd17b93f093afc00a9b906c53..e4130459b25df211ec41445bbddbd18f7bac633f 100644 (file)
--- 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 29ed05485944e7b795669d8221545a61e162ce8a..3f44b88733625191ecbd80c8a1601785fc502a57 100644 (file)
--- 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