patch 9.0.1821: Vim9 constructors are always staticv9.0.1821

Problem:  Vim9 constructors are always static
Solution: make the "static" keyword an error

closes: #12945

Signed-off-by: Christian Brabandt <cb@256bit.org>
Co-authored-by: Gianmaria Bajo <mg1979.git@gmail.com>

5 files changed, 52 insertions, 16 deletions

diff --git a/runtime/doc/vim9class.txt b/runtime/doc/vim9class.txt
index 926638bad5..8a9e37e812 100644
--- a/runtime/doc/vim9class.txt
+++ b/runtime/doc/vim9class.txt
@@ -315,6 +315,10 @@ as the first character in the method name: >
 	    OtherThing._Foo()
 	enddef
     endclass
+<
+							*E1370*
+Note that constructors cannot be declared as "static", because they always
+are.
 
 ==============================================================================
 
diff --git a/src/errors.h b/src/errors.h
index bf994333e8..1ea24600b6 100644
--- a/src/errors.h
+++ b/src/errors.h
@@ -3493,6 +3493,8 @@ EXTERN char e_static_cannot_be_followed_by_this[]
 	INIT(= N_("E1368: Static cannot be followed by \"this\" in a member name"));
 EXTERN char e_duplicate_member_str[]
 	INIT(= N_("E1369: Duplicate member: %s"));
+EXTERN char e_cannot_define_new_function_as_static[]
+	INIT(= N_("E1370: Cannot define a \"new\" function as static"));
 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[]
@@ -3510,4 +3512,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"));
 
-// E1368 - E1399 unused
+// E1371 - E1399 unused
diff --git a/src/testdir/test_vim9_class.vim b/src/testdir/test_vim9_class.vim
index 319eb0546d..184e96de0c 100644
--- a/src/testdir/test_vim9_class.vim
+++ b/src/testdir/test_vim9_class.vim
@@ -1317,6 +1317,18 @@ def Test_class_defcompile()
   END
   v9.CheckScriptFailure(lines, 'E1012: Type mismatch; expected number but got string')
 
+  lines =<< trim END
+      vim9script
+
+      class C
+          static def new()
+          enddef
+      endclass
+
+      defcompile C.new
+  END
+  v9.CheckScriptFailure(lines, 'E1370: Cannot define a "new" function as static')
+
   # Trying to compile a function using a non-existing class variable
   lines =<< trim END
     vim9script
diff --git a/src/version.c b/src/version.c
index c23c2575c0..8f978f92b1 100644
--- a/src/version.c
+++ b/src/version.c
@@ -700,6 +700,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    1821,
+/**/
     1820,
 /**/
     1819,
diff --git a/src/vim9class.c b/src/vim9class.c
index 955bd0cbcd..898dcbd4f2 100644
--- a/src/vim9class.c
+++ b/src/vim9class.c
@@ -536,6 +536,34 @@ is_duplicate_method(garray_T *fgap, char_u *name)
 }
 
 /*
+ * Returns TRUE if the constructor is valid.
+ */
+    static int
+is_valid_constructor(ufunc_T *uf, int is_abstract, int has_static)
+{
+    // Constructors are not allowed in abstract classes.
+    if (is_abstract)
+    {
+	emsg(_(e_cannot_define_new_function_in_abstract_class));
+	return FALSE;
+    }
+    // A constructor is always static, no need to define it so.
+    if (has_static)
+    {
+	emsg(_(e_cannot_define_new_function_as_static));
+	return FALSE;
+    }
+    // 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));
+	return FALSE;
+    }
+    return TRUE;
+}
+
+/*
  * Update the interface class lookup table for the member index on the
  * interface to the member index in the class implementing the interface.
  * And a lookup table for the object method index on the interface
@@ -1188,25 +1216,13 @@ early_ret:
 	    {
 		char_u *name = uf->uf_name;
 		int is_new = STRNCMP(name, "new", 3) == 0;
-		if (is_new && is_abstract)
+
+		if (is_new && !is_valid_constructor(uf, is_abstract, has_static))
 		{
-		    emsg(_(e_cannot_define_new_function_in_abstract_class));
-		    success = FALSE;
 		    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.