scanner: Warn and ignore on incorrect transfer annotations

This reverts commit 232f3c831260f596e36159112292897962a505b4.

7 files changed, 117 insertions, 34 deletions

diff --git a/giscanner/ast.py b/giscanner/ast.py
index c7ea2d74..405a71fd 100644
--- a/giscanner/ast.py
+++ b/giscanner/ast.py
@@ -214,13 +214,17 @@ TYPE_FILENAME = Type(target_fundamental='filename', ctype='gchar*')
 
 TYPE_VALIST = Type(target_fundamental='va_list', ctype='va_list')
 
-BASIC_GIR_TYPES = [TYPE_BOOLEAN, TYPE_INT8, TYPE_UINT8, TYPE_INT16,
-                   TYPE_UINT16, TYPE_INT32, TYPE_UINT32, TYPE_INT64,
-                   TYPE_UINT64, TYPE_CHAR, TYPE_SHORT, TYPE_USHORT, TYPE_INT,
-                   TYPE_UINT, TYPE_LONG, TYPE_ULONG, TYPE_SIZE, TYPE_SSIZE,
-                   TYPE_LONG_LONG, TYPE_LONG_ULONG, TYPE_INTPTR, TYPE_UINTPTR,
-                   TYPE_FLOAT, TYPE_DOUBLE,
-                   TYPE_LONG_DOUBLE, TYPE_UNICHAR, TYPE_GTYPE]
+BASIC_TYPES = [TYPE_BOOLEAN, TYPE_INT8, TYPE_UINT8, TYPE_INT16,
+               TYPE_UINT16, TYPE_INT32, TYPE_UINT32, TYPE_INT64,
+               TYPE_UINT64, TYPE_CHAR, TYPE_SHORT, TYPE_USHORT, TYPE_INT,
+               TYPE_UINT, TYPE_LONG, TYPE_ULONG, TYPE_SIZE, TYPE_SSIZE,
+               TYPE_LONG_LONG, TYPE_LONG_ULONG,
+               TYPE_FLOAT, TYPE_DOUBLE,
+               TYPE_LONG_DOUBLE, TYPE_UNICHAR, TYPE_GTYPE]
+
+BASIC_GIR_TYPES = [TYPE_INTPTR, TYPE_UINTPTR]
+BASIC_GIR_TYPES.extend(BASIC_TYPES)
+
 GIR_TYPES = [TYPE_NONE, TYPE_ANY]
 GIR_TYPES.extend(BASIC_GIR_TYPES)
 GIR_TYPES.extend([TYPE_STRING, TYPE_FILENAME, TYPE_VALIST])
diff --git a/giscanner/codegen.py b/giscanner/codegen.py
index e0eb182f..fcf1fc51 100644
--- a/giscanner/codegen.py
+++ b/giscanner/codegen.py
@@ -84,28 +84,30 @@ class CCodeGenerator(object):
         self._write_prelude(self.out_h, func)
         self.out_h.write(";\n\n")
 
-    def _write_annotation_transfer(self, transfer):
-        self.out_c.write("(transfer %s)" % (transfer, ))
+    def _write_annotation_transfer(self, node):
+        if (node.type not in ast.BASIC_TYPES or
+                node.type.ctype.endswith('*')):
+            self.out_c.write(" (transfer %s)" % (node.transfer, ))
 
     def _write_docs(self, func):
         self.out_c.write("/**\n * %s:\n" % (func.symbol, ))
         for param in func.parameters:
-            self.out_c.write(" * @%s: " % (param.argname, ))
+            self.out_c.write(" * @%s" % (param.argname, ))
             if param.direction in (ast.PARAM_DIRECTION_OUT,
                                    ast.PARAM_DIRECTION_INOUT):
                 if param.caller_allocates:
                     allocate_string = ' caller-allocates'
                 else:
                     allocate_string = ''
-                self.out_c.write("(%s%s) " % (param.direction,
-                                              allocate_string))
-                self._write_annotation_transfer(param.transfer)
+                self.out_c.write(": (%s%s) " % (param.direction,
+                                                allocate_string))
+                self._write_annotation_transfer(param)
             self.out_c.write(":\n")
         self.out_c.write(' *\n')
         self.out_c.write(' * Undocumented.\n')
         self.out_c.write(' *\n')
-        self.out_c.write(' * Returns: ')
-        self._write_annotation_transfer(func.retval.transfer)
+        self.out_c.write(' * Returns:')
+        self._write_annotation_transfer(func.retval)
         self.out_c.write('\n */')
 
     @contextmanager
diff --git a/giscanner/maintransformer.py b/giscanner/maintransformer.py
index b138a121..6c0429da 100644
--- a/giscanner/maintransformer.py
+++ b/giscanner/maintransformer.py
@@ -30,7 +30,7 @@ from .annotationparser import (ANN_ALLOW_NONE, ANN_ARRAY, ANN_ATTRIBUTES, ANN_CL
                                ANN_VFUNC, ANN_NULLABLE, ANN_OPTIONAL)
 from .annotationparser import (OPT_ARRAY_FIXED_SIZE, OPT_ARRAY_LENGTH, OPT_ARRAY_ZERO_TERMINATED,
                                OPT_OUT_CALLEE_ALLOCATES, OPT_OUT_CALLER_ALLOCATES,
-                               OPT_TRANSFER_FLOATING, OPT_TRANSFER_NONE)
+                               OPT_TRANSFER_CONTAINER, OPT_TRANSFER_FLOATING, OPT_TRANSFER_NONE)
 
 from .utils import to_underscores_noprefix
 
@@ -467,7 +467,7 @@ class MainTransformer(object):
     def _get_transfer_default_returntype_basic(self, typeval):
         if (typeval.is_equiv(ast.BASIC_GIR_TYPES)
         or typeval.is_const
-        or typeval.is_equiv(ast.TYPE_NONE)):
+        or typeval.is_equiv((ast.TYPE_ANY, ast.TYPE_NONE))):
             return ast.PARAM_TRANSFER_NONE
         elif typeval.is_equiv(ast.TYPE_STRING):
             # Non-const strings default to FULL
@@ -537,6 +537,62 @@ class MainTransformer(object):
         else:
             raise AssertionError(node)
 
+    def _is_pointer_type(self, node, annotations):
+        if (not isinstance(node, ast.Return) and
+                node.direction in (ast.PARAM_DIRECTION_OUT,
+                                   ast.PARAM_DIRECTION_INOUT)):
+            return True
+
+        target = self._transformer.lookup_typenode(node.type)
+        target = self._transformer.resolve_aliases(target)
+        target = node.type if target is None else target
+
+        return (not isinstance(target, ast.Type) or
+                target not in ast.BASIC_TYPES or
+                target.ctype.endswith('*'))
+
+    def _apply_transfer_annotation(self, parent, node, annotations):
+        transfer_annotation = annotations.get(ANN_TRANSFER)
+        if not transfer_annotation or len(transfer_annotation) != 1:
+            return
+
+        transfer = transfer_annotation[0]
+
+        target = self._transformer.lookup_typenode(node.type)
+        target = self._transformer.resolve_aliases(target)
+        target = node.type if target is None else target
+        node_type = target if isinstance(target, ast.Type) else node.type
+
+        if transfer == OPT_TRANSFER_FLOATING:
+            transfer = OPT_TRANSFER_NONE
+
+            if not isinstance(target, (ast.Class, ast.Interface)):
+                message.warn('invalid "transfer" annotation: '
+                             'only valid for object and interface types',
+                             annotations.position)
+                return
+
+        elif transfer == OPT_TRANSFER_CONTAINER:
+            if (ANN_ARRAY not in annotations and
+                    not isinstance(target, (ast.Array, ast.List, ast.Map))):
+                message.warn('invalid "transfer" annotation: '
+                             'only valid for container types',
+                             annotations.position)
+                return
+
+        elif (not self._is_pointer_type(node, annotations) and
+              node_type not in (ast.TYPE_STRING, ast.TYPE_FILENAME) and
+              not isinstance(target, (ast.Array, ast.List, ast.Map,
+                                      ast.Record, ast.Compound, ast.Boxed,
+                                      ast.Class, ast.Interface))):
+            message.warn('invalid "transfer" annotation: '
+                         'only valid for array, struct, union, boxed, '
+                         'object and interface types',
+                         annotations.position)
+            return
+
+        node.transfer = transfer
+
     def _apply_annotations_param_ret_common(self, parent, node, tag):
         annotations = tag.annotations if tag else {}
 
@@ -577,13 +633,7 @@ class MainTransformer(object):
             # Also reset the transfer default if we're toggling direction
             node.transfer = self._get_transfer_default(parent, node)
 
-        transfer_annotation = annotations.get(ANN_TRANSFER)
-        if transfer_annotation and len(transfer_annotation) == 1:
-            transfer = transfer_annotation[0]
-            if transfer == OPT_TRANSFER_FLOATING:
-                transfer = OPT_TRANSFER_NONE
-            node.transfer = transfer
-
+        self._apply_transfer_annotation(parent, node, annotations)
         self._adjust_container_type(parent, node, annotations)
 
         if ANN_NULLABLE in annotations:
diff --git a/giscanner/testcodegen.py b/giscanner/testcodegen.py
index 32139e3b..5080ed1b 100644
--- a/giscanner/testcodegen.py
+++ b/giscanner/testcodegen.py
@@ -65,6 +65,9 @@ class EverythingCodeGenerator(object):
                                   include_last_src)
 
     def write(self):
+        types = [ast.TYPE_ANY]
+        types.extend(ast.INTROSPECTABLE_BASIC)
+
         func = ast.Function('nullfunc',
                             ast.Return(ast.TYPE_NONE, transfer=ast.PARAM_TRANSFER_NONE),
                             [], False, self.gen.gen_symbol('nullfunc'))
@@ -74,7 +77,7 @@ class EverythingCodeGenerator(object):
 
         # First pass, generate constant returns
         prefix = 'const return '
-        for typeval in ast.INTROSPECTABLE_BASIC:
+        for typeval in types:
             name = prefix + uscore_from_type(typeval)
             sym = self.gen.gen_symbol(name)
             func = ast.Function(name,
@@ -87,7 +90,7 @@ class EverythingCodeGenerator(object):
 
         # Void return, one parameter
         prefix = 'oneparam '
-        for typeval in ast.INTROSPECTABLE_BASIC:
+        for typeval in types:
             if typeval is ast.TYPE_NONE:
                 continue
             name = prefix + uscore_from_type(typeval)
@@ -101,7 +104,7 @@ class EverythingCodeGenerator(object):
 
         # Void return, one (out) parameter
         prefix = 'one_outparam '
-        for typeval in ast.INTROSPECTABLE_BASIC:
+        for typeval in types:
             if typeval is ast.TYPE_NONE:
                 continue
             name = prefix + uscore_from_type(typeval)
@@ -119,7 +122,7 @@ class EverythingCodeGenerator(object):
 
         # Passthrough one parameter
         prefix = 'passthrough_one '
-        for typeval in ast.INTROSPECTABLE_BASIC:
+        for typeval in types:
             if typeval is ast.TYPE_NONE:
                 continue
             name = prefix + uscore_from_type(typeval)
diff --git a/tests/gimarshallingtests.c b/tests/gimarshallingtests.c
index 76beb92e..69311de7 100644
--- a/tests/gimarshallingtests.c
+++ b/tests/gimarshallingtests.c
@@ -3409,7 +3409,7 @@ glong
 /**
  * gi_marshalling_tests_pointer_in_return:
  *
- * Returns: (transfer none): The same pointer
+ * Returns: The same pointer
  */
 gpointer
 gi_marshalling_tests_pointer_in_return (gpointer pointer)
diff --git a/tests/scanner/Regress-1.0-expected.gir b/tests/scanner/Regress-1.0-expected.gir
index aeb3fdbc..bc3d29dc 100644
--- a/tests/scanner/Regress-1.0-expected.gir
+++ b/tests/scanner/Regress-1.0-expected.gir
@@ -1461,7 +1461,7 @@ uses a C sugar return type.</doc>
               c:identifier="regress_foo_object_new_cookie"
               introspectable="0">
         <doc xml:space="preserve">Not sure why this test is here...</doc>
-        <return-value>
+        <return-value transfer-ownership="none">
           <type name="FooObjectCookie" c:type="RegressFooObjectCookie"/>
         </return-value>
         <parameters>
diff --git a/tests/warn/invalid-transfer.h b/tests/warn/invalid-transfer.h
index ec43f2ac..054bb768 100644
--- a/tests/warn/invalid-transfer.h
+++ b/tests/warn/invalid-transfer.h
@@ -1,12 +1,36 @@
+#include "common.h"
+
+typedef char TestChar;
 
 /**
  * test_transfer_invalid:
  * @param: (transfer):
  * @param2: (transfer invalid):
  * @param3: (transfer full foo):
+ * @param4: (transfer full):
+ * @param5: (transfer full):
+ * @param6: (transfer full):
+ * @param7: (transfer container):
+ * @param8: (transfer floating):
+ */
+void test_transfer_invalid(GObject *param, GObject *param2, GObject *param3,
+                           char param4, TestChar param5, GType param6,
+                           GObject *param7, GDateTime *param8);
+
+// EXPECT:7: Warning: Test: "transfer" annotation needs one option, none given
+// EXPECT:8: Warning: Test: invalid "transfer" annotation option: "invalid"
+// EXPECT:9: Warning: Test: "transfer" annotation needs one option, 2 given
+// EXPECT:10: Warning: Test: invalid "transfer" annotation: only valid for array, struct, union, boxed, object and interface types
+// EXPECT:11: Warning: Test: invalid "transfer" annotation: only valid for array, struct, union, boxed, object and interface types
+// EXPECT:12: Warning: Test: invalid "transfer" annotation: only valid for array, struct, union, boxed, object and interface types
+// EXPECT:13: Warning: Test: invalid "transfer" annotation: only valid for container types
+// EXPECT:14: Warning: Test: invalid "transfer" annotation: only valid for object and interface types
+
+/**
+ * test_transfer_return_invalid:
+ *
+ * Returns: (transfer full):
  */
-void test_transfer_invalid(int param, int param2, int param3);
+char test_transfer_return_invalid (void);
 
-// EXPECT:4: Warning: Test: "transfer" annotation needs one option, none given
-// EXPECT:5: Warning: Test: invalid "transfer" annotation option: "invalid"
-// EXPECT:6: Warning: Test: "transfer" annotation needs one option, 2 given
+// EXPECT:32: Warning: Test: invalid "transfer" annotation: only valid for array, struct, union, boxed, object and interface types