giscanner: Mark gpointer nodes as nullable by default

gpointer parameters and return types should be marked as nullable by
default, unless:
 • also annotated with (type) and not with (nullable); or
 • explicitly annotated with (not nullable).

This introduces the (not nullable) annotation as a direct opposite to
(nullable). In future, (not) could be extended to invert other
annotations.

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

9 files changed, 184 insertions, 29 deletions

diff --git a/giscanner/annotationparser.py b/giscanner/annotationparser.py
index d48ed616..31a62927 100644
--- a/giscanner/annotationparser.py
+++ b/giscanner/annotationparser.py
@@ -208,6 +208,7 @@ ANN_INOUT = 'inout'
 ANN_METHOD = 'method'
 ANN_NULLABLE = 'nullable'
 ANN_OPTIONAL = 'optional'
+ANN_NOT = 'not'
 ANN_OUT = 'out'
 ANN_REF_FUNC = 'ref-func'
 ANN_RENAME_TO = 'rename-to'
@@ -223,6 +224,7 @@ ANN_VALUE = 'value'
 GI_ANNS = [ANN_ALLOW_NONE,
            ANN_NULLABLE,
            ANN_OPTIONAL,
+           ANN_NOT,
            ANN_ARRAY,
            ANN_ATTRIBUTES,
            ANN_CLOSURE,
@@ -273,6 +275,11 @@ OPT_OUT_CALLER_ALLOCATES = 'caller-allocates'
 OUT_OPTIONS = [OPT_OUT_CALLEE_ALLOCATES,
                OPT_OUT_CALLER_ALLOCATES]
 
+# (not) annotation options
+OPT_NOT_NULLABLE = 'nullable'
+
+NOT_OPTIONS = [OPT_NOT_NULLABLE]
+
 # (scope) annotation options
 OPT_SCOPE_ASYNC = 'async'
 OPT_SCOPE_CALL = 'call'
@@ -568,6 +575,18 @@ class GtkDocAnnotatable(object):
                     # GObject-Instrospection version.
                     warn('unknown annotation: %s' % (ann_name, ), position)
 
+                # Validate that (nullable) and (not nullable) are not both
+                # present. Same for (allow-none) and (not nullable).
+                if ann_name == ANN_NOT and OPT_NOT_NULLABLE in options:
+                    if ANN_NULLABLE in self.annotations:
+                        warn('cannot have both "%s" and "%s" present' %
+                             (ANN_NOT + ' ' + OPT_NOT_NULLABLE, ANN_NULLABLE),
+                             position)
+                    if ANN_ALLOW_NONE in self.annotations:
+                        warn('cannot have both "%s" and "%s" present' %
+                             (ANN_NOT + ' ' + OPT_NOT_NULLABLE, ANN_ALLOW_NONE),
+                             position)
+
     def _validate_options(self, position, ann_name, n_options, expected_n_options, operator,
                           message):
         '''
@@ -829,6 +848,19 @@ class GtkDocAnnotatable(object):
 
         self._validate_annotation(position, ann_name, options, exact_n_options=0)
 
+    def _do_validate_not(self, position, ann_name, options):
+        '''
+        Validate the ``(not)`` annotation.
+
+        :param position: :class:`giscanner.message.Position` of the line in the source file
+                         containing the annotation to be validated
+        :param ann_name: name of the annotation holding the options to validate
+        :param options: annotation options held by the annotation
+        '''
+
+        self._validate_annotation(position, ann_name, options, exact_n_options=1,
+                                  choices=NOT_OPTIONS)
+
     def _do_validate_out(self, position, ann_name, options):
         '''
         Validate the ``(out)`` annotation.
@@ -974,7 +1006,7 @@ class GtkDocParameter(GtkDocAnnotatable):
 
     valid_annotations = (ANN_ALLOW_NONE, ANN_ARRAY, ANN_ATTRIBUTES, ANN_CLOSURE, ANN_DESTROY,
                          ANN_ELEMENT_TYPE, ANN_IN, ANN_INOUT, ANN_OUT, ANN_SCOPE, ANN_SKIP,
-                         ANN_TRANSFER, ANN_TYPE, ANN_OPTIONAL, ANN_NULLABLE)
+                         ANN_TRANSFER, ANN_TYPE, ANN_OPTIONAL, ANN_NULLABLE, ANN_NOT)
 
     def __init__(self, name, position=None):
         GtkDocAnnotatable.__init__(self, position)
@@ -997,7 +1029,7 @@ class GtkDocTag(GtkDocAnnotatable):
     __slots__ = ('name', 'value', 'description')
 
     valid_annotations = (ANN_ALLOW_NONE, ANN_ARRAY, ANN_ATTRIBUTES, ANN_ELEMENT_TYPE, ANN_SKIP,
-                         ANN_TRANSFER, ANN_TYPE, ANN_NULLABLE, ANN_OPTIONAL)
+                         ANN_TRANSFER, ANN_TYPE, ANN_NULLABLE, ANN_OPTIONAL, ANN_NOT)
 
     def __init__(self, name, position=None):
         GtkDocAnnotatable.__init__(self, position)
diff --git a/giscanner/ast.py b/giscanner/ast.py
index aeb49ed2..99bbd3e1 100644
--- a/giscanner/ast.py
+++ b/giscanner/ast.py
@@ -845,10 +845,11 @@ class Alias(Node):
 class TypeContainer(Annotated):
     """A fundamental base class for Return and Parameter."""
 
-    def __init__(self, typenode, nullable, transfer, direction):
+    def __init__(self, typenode, nullable, not_nullable, transfer, direction):
         Annotated.__init__(self)
         self.type = typenode
         self.nullable = nullable
+        self.not_nullable = not_nullable
         self.direction = direction
         if transfer is not None:
             self.transfer = transfer
@@ -864,8 +865,9 @@ class Parameter(TypeContainer):
     def __init__(self, argname, typenode, direction=None,
                  transfer=None, nullable=False, optional=False,
                  allow_none=False, scope=None,
-                 caller_allocates=False):
-        TypeContainer.__init__(self, typenode, nullable, transfer, direction)
+                 caller_allocates=False, not_nullable=False):
+        TypeContainer.__init__(self, typenode, nullable, not_nullable,
+                               transfer, direction)
         self.argname = argname
         self.optional = optional
         self.parent = None  # A Callable
@@ -889,8 +891,9 @@ class Parameter(TypeContainer):
 class Return(TypeContainer):
     """A return value from a function."""
 
-    def __init__(self, rtype, nullable=False, transfer=None):
-        TypeContainer.__init__(self, rtype, nullable, transfer,
+    def __init__(self, rtype, nullable=False, not_nullable=False,
+                 transfer=None):
+        TypeContainer.__init__(self, rtype, nullable, not_nullable, transfer,
                                direction=PARAM_DIRECTION_OUT)
         self.parent = None  # A Callable
 
diff --git a/giscanner/girparser.py b/giscanner/girparser.py
index 57f3e908..909e08d9 100644
--- a/giscanner/girparser.py
+++ b/giscanner/girparser.py
@@ -318,7 +318,7 @@ class GIRParser(object):
             raise ValueError('node %r has no return-value' % (name, ))
         transfer = returnnode.attrib.get('transfer-ownership')
         nullable = returnnode.attrib.get('nullable') == '1'
-        retval = ast.Return(self._parse_type(returnnode), nullable, transfer)
+        retval = ast.Return(self._parse_type(returnnode), nullable, False, transfer)
         self._parse_generic_attribs(returnnode, retval)
         parameters = []
 
diff --git a/giscanner/girwriter.py b/giscanner/girwriter.py
index e73dcaca..f637376f 100644
--- a/giscanner/girwriter.py
+++ b/giscanner/girwriter.py
@@ -214,7 +214,7 @@ class GIRWriter(XMLWriter):
             attrs.append(('transfer-ownership', return_.transfer))
         if return_.skip:
             attrs.append(('skip', '1'))
-        if return_.nullable:
+        if return_.nullable and not return_.not_nullable:
             attrs.append(('nullable', '1'))
         with self.tagcontext('return-value', attrs):
             self._write_generic(return_)
@@ -240,7 +240,7 @@ class GIRWriter(XMLWriter):
         if parameter.transfer:
             attrs.append(('transfer-ownership',
                           parameter.transfer))
-        if parameter.nullable:
+        if parameter.nullable and not parameter.not_nullable:
             attrs.append(('nullable', '1'))
             if parameter.direction != ast.PARAM_DIRECTION_OUT:
                 attrs.append(('allow-none', '1'))
diff --git a/giscanner/maintransformer.py b/giscanner/maintransformer.py
index 8bac1430..5ea1f5b4 100644
--- a/giscanner/maintransformer.py
+++ b/giscanner/maintransformer.py
@@ -32,10 +32,11 @@ from .annotationparser import (ANN_ALLOW_NONE, ANN_ARRAY, ANN_ATTRIBUTES, ANN_CL
                                ANN_GET_VALUE_FUNC, ANN_IN, ANN_INOUT, ANN_METHOD, ANN_OUT,
                                ANN_REF_FUNC, ANN_RENAME_TO, ANN_SCOPE, ANN_SET_VALUE_FUNC,
                                ANN_SKIP, ANN_TRANSFER, ANN_TYPE, ANN_UNREF_FUNC, ANN_VALUE,
-                               ANN_VFUNC, ANN_NULLABLE, ANN_OPTIONAL)
+                               ANN_VFUNC, ANN_NULLABLE, ANN_OPTIONAL, ANN_NOT)
 from .annotationparser import (OPT_ARRAY_FIXED_SIZE, OPT_ARRAY_LENGTH, OPT_ARRAY_ZERO_TERMINATED,
                                OPT_OUT_CALLEE_ALLOCATES, OPT_OUT_CALLER_ALLOCATES,
-                               OPT_TRANSFER_CONTAINER, OPT_TRANSFER_FLOATING, OPT_TRANSFER_NONE)
+                               OPT_TRANSFER_CONTAINER, OPT_TRANSFER_FLOATING, OPT_TRANSFER_NONE,
+                               OPT_NOT_NULLABLE)
 
 from .utils import to_underscores_noprefix
 
@@ -646,9 +647,16 @@ class MainTransformer(object):
         self._apply_transfer_annotation(parent, node, annotations)
         self._adjust_container_type(parent, node, annotations)
 
+        # gpointer parameters and return values are always nullable unless:
+        #  - annotated with (type) and not also with (nullable); or
+        #  - annotated (not nullable)
+        # See: https://bugzilla.gnome.org/show_bug.cgi?id=719966#c22
+        if node.type.is_equiv(ast.TYPE_ANY):
+            node.nullable = True
         if ANN_NULLABLE in annotations:
             if self._is_pointer_type(node, annotations):
                 node.nullable = True
+                node.not_nullable = False
             else:
                 message.warn('invalid "nullable" annotation: '
                              'only valid for pointer types and out parameters',
@@ -679,6 +687,11 @@ class MainTransformer(object):
                  node.type.target_giname == 'Gio.Cancellable')):
             node.nullable = True
 
+        # Final override for nullability
+        if ANN_NOT in annotations:
+            node.nullable = False
+            node.not_nullable = True
+
         if tag and tag.description:
             node.doc = tag.description
 
@@ -1443,7 +1456,8 @@ method or constructor of some type."""
         for param in params:
             # By convention, closure user_data parameters are always nullable.
             if param.closure_name is not None and \
-               param.closure_name == param.argname:
+               param.closure_name == param.argname and \
+               not param.not_nullable:
                 param.nullable = True
 
     def _pass3_callable_throws(self, node):
diff --git a/tests/scanner/Regress-1.0-expected.gir b/tests/scanner/Regress-1.0-expected.gir
index b856867f..dfbbd8d5 100644
--- a/tests/scanner/Regress-1.0-expected.gir
+++ b/tests/scanner/Regress-1.0-expected.gir
@@ -790,7 +790,10 @@ regress_annotation_object_watch_full().</doc>
           <instance-parameter name="object" transfer-ownership="none">
             <type name="AnnotationObject" c:type="RegressAnnotationObject*"/>
           </instance-parameter>
-          <parameter name="data" transfer-ownership="none">
+          <parameter name="data"
+                     transfer-ownership="none"
+                     nullable="1"
+                     allow-none="1">
             <doc xml:space="preserve">Opaque pointer handle</doc>
             <type name="gpointer" c:type="void*"/>
           </parameter>
@@ -851,7 +854,10 @@ of tabs and strings to test the tab handling capabilities of the scanner.</doc>
           <type name="none" c:type="void"/>
         </return-value>
         <parameters>
-          <parameter name="arg1" transfer-ownership="none">
+          <parameter name="arg1"
+                     transfer-ownership="none"
+                     nullable="1"
+                     allow-none="1">
             <type name="gpointer" c:type="gpointer"/>
           </parameter>
         </parameters>
@@ -1071,7 +1077,10 @@ it says it's pointer but it's actually a string.</doc>
         <parameter name="b" transfer-ownership="none">
           <type name="gboolean" c:type="gboolean"/>
         </parameter>
-        <parameter name="data" transfer-ownership="none">
+        <parameter name="data"
+                   transfer-ownership="none"
+                   nullable="1"
+                   allow-none="1">
           <type name="gpointer" c:type="gpointer"/>
         </parameter>
       </parameters>
@@ -1553,7 +1562,10 @@ uses a C sugar return type.</doc>
           <instance-parameter name="object" transfer-ownership="none">
             <type name="FooObject" c:type="RegressFooObject*"/>
           </instance-parameter>
-          <parameter name="data" transfer-ownership="none">
+          <parameter name="data"
+                     transfer-ownership="none"
+                     nullable="1"
+                     allow-none="1">
             <type name="gpointer" c:type="void*"/>
           </parameter>
           <parameter name="some_type" transfer-ownership="none">
@@ -1602,7 +1614,10 @@ uses a C sugar return type.</doc>
           <parameter name="object" transfer-ownership="none">
             <type name="GObject.Object"/>
           </parameter>
-          <parameter name="p0" transfer-ownership="none">
+          <parameter name="p0"
+                     transfer-ownership="none"
+                     nullable="1"
+                     allow-none="1">
             <type name="gpointer" c:type="gpointer"/>
           </parameter>
         </parameters>
@@ -5486,11 +5501,7 @@ call and can be released on return.</doc>
           <type name="TestCallbackUserData"
                 c:type="RegressTestCallbackUserData"/>
         </parameter>
-        <parameter name="user_data"
-                   transfer-ownership="none"
-                   nullable="1"
-                   allow-none="1"
-                   closure="1">
+        <parameter name="user_data" transfer-ownership="none" closure="1">
           <type name="gpointer" c:type="gpointer"/>
         </parameter>
       </parameters>
diff --git a/tests/scanner/WarnLib-1.0-expected.gir b/tests/scanner/WarnLib-1.0-expected.gir
index 47e3666c..007ec864 100644
--- a/tests/scanner/WarnLib-1.0-expected.gir
+++ b/tests/scanner/WarnLib-1.0-expected.gir
@@ -33,7 +33,10 @@ and/or use gtk-doc annotations.  -->
             <doc xml:space="preserve">x parameter</doc>
             <type name="gint" c:type="int"/>
           </parameter>
-          <parameter name="y" transfer-ownership="none">
+          <parameter name="y"
+                     transfer-ownership="none"
+                     nullable="1"
+                     allow-none="1">
             <doc xml:space="preserve">y parameter</doc>
             <type name="gpointer" c:type="gpointer"/>
           </parameter>
@@ -50,7 +53,10 @@ and/or use gtk-doc annotations.  -->
           <parameter name="arg1" transfer-ownership="none">
             <type name="gint" c:type="int"/>
           </parameter>
-          <parameter name="arg2" transfer-ownership="none">
+          <parameter name="arg2"
+                     transfer-ownership="none"
+                     nullable="1"
+                     allow-none="1">
             <type name="gpointer" c:type="gpointer"/>
           </parameter>
         </parameters>
@@ -68,7 +74,10 @@ and/or use gtk-doc annotations.  -->
           <parameter name="arg1" transfer-ownership="none">
             <type name="gint" c:type="int"/>
           </parameter>
-          <parameter name="arg2" transfer-ownership="none">
+          <parameter name="arg2"
+                     transfer-ownership="none"
+                     nullable="1"
+                     allow-none="1">
             <type name="gpointer" c:type="gpointer"/>
           </parameter>
         </parameters>
@@ -84,7 +93,10 @@ and/or use gtk-doc annotations.  -->
           <parameter name="arg1" transfer-ownership="none">
             <type name="gint" c:type="int"/>
           </parameter>
-          <parameter name="arg2" transfer-ownership="none">
+          <parameter name="arg2"
+                     transfer-ownership="none"
+                     nullable="1"
+                     allow-none="1">
             <type name="gpointer" c:type="gpointer"/>
           </parameter>
         </parameters>
@@ -108,7 +120,10 @@ and/or use gtk-doc annotations.  -->
             <parameter name="arg1" transfer-ownership="none">
               <type name="gint" c:type="int"/>
             </parameter>
-            <parameter name="arg2" transfer-ownership="none">
+            <parameter name="arg2"
+                       transfer-ownership="none"
+                       nullable="1"
+                       allow-none="1">
               <type name="gpointer" c:type="gpointer"/>
             </parameter>
           </parameters>
@@ -128,7 +143,10 @@ and/or use gtk-doc annotations.  -->
               <doc xml:space="preserve">x parameter</doc>
               <type name="gint" c:type="int"/>
             </parameter>
-            <parameter name="y" transfer-ownership="none">
+            <parameter name="y"
+                       transfer-ownership="none"
+                       nullable="1"
+                       allow-none="1">
               <doc xml:space="preserve">y parameter</doc>
               <type name="gpointer" c:type="gpointer"/>
             </parameter>
diff --git a/tests/scanner/annotationparser/gi/annotation_not_nullable.xml b/tests/scanner/annotationparser/gi/annotation_not_nullable.xml
new file mode 100644
index 00000000..d63ec94e
--- /dev/null
+++ b/tests/scanner/annotationparser/gi/annotation_not_nullable.xml
@@ -0,0 +1,76 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<tests xmlns="http://schemas.gnome.org/gobject-introspection/2013/test">
+
+<test>
+  <input>/**
+ * annotation_object_not_nullable:
+ * @object: a #GObject
+ * @closurearg: (closure) (not nullable): This is an argument test
+ *
+ * This is a test for not-nullable arguments which would otherwise be nullable
+ * by convention.
+ *
+ * Return value: (not nullable): a pointer
+ */</input>
+  <parser>
+    <docblock>
+      <identifier>
+        <name>annotation_object_not_nullable</name>
+      </identifier>
+      <parameters>
+        <parameter>
+          <name>object</name>
+          <description>a #GObject</description>
+        </parameter>
+        <parameter>
+          <name>closurearg</name>
+          <annotations>
+            <annotation>
+              <name>closure</name>
+            </annotation>
+            <annotation>
+              <name>not</name>
+              <options>
+                <option>
+                  <name>nullable</name>
+                </option>
+              </options>
+            </annotation>
+          </annotations>
+          <description>This is an argument test</description>
+        </parameter>
+      </parameters>
+      <description>This is a test for not-nullable arguments which would otherwise be nullable
+by convention.</description>
+      <tags>
+        <tag>
+          <name>returns</name>
+          <annotations>
+            <annotation>
+              <name>not</name>
+              <options>
+                <option>
+                  <name>nullable</name>
+                </option>
+              </options>
+            </annotation>
+          </annotations>
+          <description>a pointer</description>
+        </tag>
+      </tags>
+    </docblock>
+  </parser>
+  <output>/**
+ * annotation_object_not_nullable:
+ * @object: a #GObject
+ * @closurearg: (closure) (not nullable): This is an argument test
+ *
+ * This is a test for not-nullable arguments which would otherwise be nullable
+ * by convention.
+ *
+ * Returns: (not nullable): a pointer
+ */</output>
+</test>
+
+</tests>
diff --git a/tests/scanner/regress.c b/tests/scanner/regress.c
index 537d7d66..3644f4ca 100644
--- a/tests/scanner/regress.c
+++ b/tests/scanner/regress.c
@@ -3478,6 +3478,7 @@ regress_test_simple_callback (RegressTestSimpleCallback callback)
 /**
  * regress_test_callback_user_data:
  * @callback: (scope call):
+ * @user_data: (not nullable):
  *
  * Call - callback parameter persists for the duration of the method
  * call and can be released on return.