Add warnings for the (constructor) annotation

https://bugzilla.gnome.org/show_bug.cgi?id=641347

6 files changed, 85 insertions, 11 deletions

diff --git a/giscanner/maintransformer.py b/giscanner/maintransformer.py
index 38642081..f8bf9f1e 100644
--- a/giscanner/maintransformer.py
+++ b/giscanner/maintransformer.py
@@ -897,7 +897,7 @@ method or constructor of some type."""
             return
         (ns, subsymbol) = self._transformer.split_csymbol(func.symbol)
         assert ns == self._namespace
-        if func.is_constructor or self._is_constructor(func, subsymbol):
+        if self._is_constructor(func, subsymbol):
             self._set_up_constructor(func, subsymbol)
             return
         elif self._is_method(func, subsymbol):
@@ -1002,31 +1002,66 @@ method or constructor of some type."""
     def _set_up_constructor(self, func, subsymbol):
         self._namespace.float(func)
 
-        origin_node, funcname = self._split_uscored_by_type(subsymbol)
-        func.name = funcname
+        func.name = self._get_constructor_name(func, subsymbol)
+
+        origin_node = self._get_constructor_class(func, subsymbol)
         origin_node.constructors.append(func)
 
         func.is_constructor = True
 
         # Constructors have default return semantics
         if not func.retval.transfer:
-            func.retval.transfer = self._get_transfer_default_return(func, func.retval)
+            func.retval.transfer = self._get_transfer_default_return(func,
+                    func.retval)
+
+    def _get_constructor_class(self, func, subsymbol):
+        origin_node = None
+        split = self._split_uscored_by_type(subsymbol)
+        if split is None:
+            if func.is_constructor:
+                origin_node = self._transformer.lookup_typenode(func.retval.type)
+        else:
+            origin_node, _ = split
+
+        return origin_node
+
+    def _get_constructor_name(self, func, subsymbol):
+        name = None
+        split = self._split_uscored_by_type(subsymbol)
+        if split is None:
+            if func.is_constructor:
+                name = func.name
+        else:
+            _, name = split
+
+        return name
 
     def _is_constructor(self, func, subsymbol):
-        if not (func.symbol.find('_new_') >= 0 or func.symbol.endswith('_new')):
+        if False and func.symbol == 'regress_constructor':
+            import pdb
+            pdb.set_trace()
+        # func.is_constructor will be True if we have a (constructor) annotation
+        if not func.is_constructor and \
+                not (func.symbol.find('_new_') >= 0 or \
+                        func.symbol.endswith('_new')):
             return False
         target = self._transformer.lookup_typenode(func.retval.type)
         if not (isinstance(target, ast.Class)
                 or (isinstance(target, (ast.Record, ast.Union, ast.Boxed))
                     and (target.get_type is not None or target.foreign))):
+            if func.is_constructor:
+                message.warn_node(func,
+                    '%s: Constructors must return an instance of their class'
+                    % (func.symbol, ))
             return False
 
-        split = self._split_uscored_by_type(subsymbol)
-        if split is None:
+        origin_node = self._get_constructor_class(func, subsymbol)
+        if origin_node is None:
             message.warn_node(func,
-                "Can't find matching type for constructor; symbol=%r" % (func.symbol, ))
+                "Can't find matching type for constructor; symbol=%r" \
+                % (func.symbol, ))
             return False
-        (origin_node, funcname) = split
+
         # Some sanity checks; only objects and boxeds can have ctors
         if not (isinstance(origin_node, ast.Class)
                 or (isinstance(origin_node, (ast.Record, ast.Union, ast.Boxed))
@@ -1034,9 +1069,13 @@ method or constructor of some type."""
             return False
         # Verify the namespace - don't want to append to foreign namespaces!
         if origin_node.namespace != self._namespace:
+            if func.is_constructor:
+                message.warn_node(func,
+                    '%s: Constructors must belong to the same namespace as the '
+                    'class they belong to' % (func.symbol, ))
             return False
-        # If it takes the object as a first arg, it's not a constructor
-        if len(func.parameters) > 0:
+        # If it takes the object as a first arg, guess it's not a constructor
+        if not func.is_constructor and len(func.parameters) > 0:
             first_arg = self._transformer.lookup_typenode(func.parameters[0].type)
             if (first_arg is not None) and first_arg.gi_name == origin_node.gi_name:
                 return False
diff --git a/tests/scanner/Regress-1.0-expected.gir b/tests/scanner/Regress-1.0-expected.gir
index 10429ac7..2bae748b 100644
--- a/tests/scanner/Regress-1.0-expected.gir
+++ b/tests/scanner/Regress-1.0-expected.gir
@@ -372,6 +372,11 @@ use it should be.</doc>
            glib:type-name="RegressTestObj"
            glib:get-type="regress_test_obj_get_type"
            glib:type-struct="TestObjClass">
+      <constructor name="constructor" c:identifier="regress_constructor">
+        <return-value transfer-ownership="full">
+          <type name="TestObj" c:type="RegressTestObj*"/>
+        </return-value>
+      </constructor>
       <constructor name="new" c:identifier="regress_test_obj_new">
         <return-value transfer-ownership="full">
           <type name="TestObj" c:type="RegressTestObj*"/>
diff --git a/tests/scanner/regress.c b/tests/scanner/regress.c
index 8b71bc8c..3ed9a862 100644
--- a/tests/scanner/regress.c
+++ b/tests/scanner/regress.c
@@ -1920,6 +1920,16 @@ regress_test_obj_new (RegressTestObj *obj)
 }
 
 /**
+ * regress_constructor: (constructor)
+ *
+ */
+RegressTestObj *
+regress_constructor (void)
+{
+  return g_object_new (REGRESS_TEST_TYPE_OBJ, NULL);
+}
+
+/**
  * regress_test_obj_new_from_file:
  */
 RegressTestObj *
diff --git a/tests/scanner/regress.h b/tests/scanner/regress.h
index 360390d4..89209ebc 100644
--- a/tests/scanner/regress.h
+++ b/tests/scanner/regress.h
@@ -298,6 +298,7 @@ struct _RegressTestObjClass
 
 GType      regress_test_obj_get_type (void);
 RegressTestObj*   regress_test_obj_new (RegressTestObj *obj);
+RegressTestObj*   regress_constructor (void);
 RegressTestObj*   regress_test_obj_new_from_file (const char *x, GError **error);
 void       regress_test_obj_set_bare (RegressTestObj *obj, GObject *bare);
 int        regress_test_obj_instance_method (RegressTestObj *obj);
diff --git a/tests/warn/Makefile.am b/tests/warn/Makefile.am
index 571ca4a3..7d81d9ef 100644
--- a/tests/warn/Makefile.am
+++ b/tests/warn/Makefile.am
@@ -6,6 +6,7 @@ TESTS = \
 	return-gobject.h \
 	invalid-array.h \
 	invalid-closure.h \
+	invalid-constructor.h \
 	invalid-element-type.h \
 	invalid-method.h \
 	invalid-option.h \
diff --git a/tests/warn/invalid-constructor.h b/tests/warn/invalid-constructor.h
new file mode 100644
index 00000000..a80d79ea
--- /dev/null
+++ b/tests/warn/invalid-constructor.h
@@ -0,0 +1,18 @@
+#include "common.h"
+
+/**
+ * test_constructor_invalid_return: (constructor)
+ *
+ */
+int test_constructor_invalid_return();
+
+// EXPECT:7: Warning: Test: test_constructor_invalid_return: Constructors must return an instance of their class
+
+/**
+ * test_constructor_invalid_return_pointer: (constructor)
+ *
+ */
+GClosure *test_constructor_invalid_return_pointer();
+
+// EXPECT:15: Warning: Test: test_constructor_invalid_return_pointer: Constructors must belong to the same namespace as the class they belong to
+