Add scope annotation value warnings

Refactor and improve the warning messages for the transfer
warnings as well

3 files changed, 64 insertions, 29 deletions

diff --git a/giscanner/annotationparser.py b/giscanner/annotationparser.py
index 58094d0d..0e8fca5d 100644
--- a/giscanner/annotationparser.py
+++ b/giscanner/annotationparser.py
@@ -80,6 +80,7 @@ OPT_ARRAY_FIXED_SIZE = 'fixed-size'
 OPT_ARRAY_LENGTH = 'length'
 OPT_ARRAY_ZERO_TERMINATED = 'zero-terminated'
 
+# Scope options
 OPT_SCOPE_ASYNC = 'async'
 OPT_SCOPE_CALL = 'call'
 OPT_SCOPE_NOTIFIED = 'notified'
@@ -125,27 +126,50 @@ class DocTag(object):
     def __repr__(self):
         return '<DocTag %r %r>' % (self.name, self.options)
 
+    def _validate_option(self, name, value, required=False, n_params=None, choices=None):
+        if required and value is None:
+            message.warn('%s annotation needs a value' % (name, ), self.position)
+            return
+
+        if n_params is not None and value.length() != n_params:
+            if n_params == 0:
+                s = 'no value'
+            elif n_params == 1:
+                s = 'one value'
+            else:
+                s = '%d values' % (n_params, )
+            message.warn('%s annotation needs %s, not %d' % (
+                name, s, value.length()), self.position)
+            return
+
+        if choices is not None:
+            valuestr = value.one()
+            if valuestr not in choices:
+                message.warn('invalid %s annotation value: %r' % (
+                    name, valuestr, ), self.position)
+                return
+
     def validate(self):
         for option in self.options:
             if not option in ALL_OPTIONS:
                 message.warn('invalid annotation option: %s' % (option, ),
                              positions=self.position)
+            value = self.options[option]
             if option == OPT_TRANSFER:
-                value = self.options[option]
-                if value is None:
-                    message.warn('transfer needs a value',
-                                 self.position)
-                    continue
-                if value.length() != 1:
-                    message.warn('transfer needs one value, not %d' % (
-                        value.length()), self.position)
-                    continue
-                valuestr = value.one()
-                if valuestr not in [OPT_TRANSFER_NONE,
-                                    OPT_TRANSFER_CONTAINER,
-                                    OPT_TRANSFER_FULL]:
-                    message.warn('invalid transfer value: %r' % (
-                        valuestr, ), self.position)
+                self._validate_option(
+                    'transfer', value, required=True,
+                    n_params=1,
+                    choices=[OPT_TRANSFER_FULL,
+                             OPT_TRANSFER_CONTAINER,
+                             OPT_TRANSFER_NONE])
+
+            elif option == OPT_SCOPE:
+                self._validate_option(
+                    'scope', value, required=True,
+                    n_params=1,
+                    choices=[OPT_SCOPE_ASYNC,
+                             OPT_SCOPE_CALL,
+                             OPT_SCOPE_NOTIFIED])
 
 class DocOptions(object):
     def __init__(self):
diff --git a/giscanner/maintransformer.py b/giscanner/maintransformer.py
index d41b55f0..a670a0a2 100644
--- a/giscanner/maintransformer.py
+++ b/giscanner/maintransformer.py
@@ -558,18 +558,9 @@ usage is void (*_gtk_reserved1)(void);"""
             options = {}
         if isinstance(parent, (ast.Function, ast.VFunction)):
             scope = options.get(OPT_SCOPE)
-            if scope:
-                scope = scope.one()
-                if scope not in [ast.PARAM_SCOPE_CALL,
-                                 ast.PARAM_SCOPE_ASYNC,
-                                 ast.PARAM_SCOPE_NOTIFIED]:
-                    message.warn(
-                        "Invalid scope %r for parameter %r" % (scope,
-                        param.argname),
-                        tag.position)
-                else:
-                    param.scope = scope
-                    param.transfer = ast.PARAM_TRANSFER_NONE
+            if scope and scope.length() == 1:
+                param.scope = scope.one()
+                param.transfer = ast.PARAM_TRANSFER_NONE
 
             destroy = options.get(OPT_DESTROY)
             if destroy:
diff --git a/tests/warn/callback-invalid-scope.h b/tests/warn/callback-invalid-scope.h
index 46292aa8..be0ef5bc 100644
--- a/tests/warn/callback-invalid-scope.h
+++ b/tests/warn/callback-invalid-scope.h
@@ -7,5 +7,25 @@
  */
 void test_callback_invalid(GCallback *callback, gpointer user_data);
 
-// EXPECT:5: Warning: Test: Invalid scope 'invalid' for parameter 'callback'
-// EXPECT:3: Warning: Test: test_callback_invalid: argument callback: Missing (scope) annotation for callback without GDestroyNotify (valid: call, async)
+// EXPECT:5: Warning: Test: invalid scope annotation value: 'invalid'
+
+/**
+ * test_callback_invalid2:
+ * @callback: (scope):
+ *
+ */
+void test_callback_invalid2(GCallback *callback, gpointer user_data);
+
+// EXPECT:14: Warning: Test: scope annotation needs a value
+
+/**
+ * test_callback_invalid3:
+ * @callback: (scope invalid foo):
+ *
+ */
+void test_callback_invalid3(GCallback *callback, gpointer user_data);
+
+// EXPECT:23: Warning: Test: scope annotation needs one value, not 2
+
+// EXPECT:12: Warning: Test: test_callback_invalid2: argument callback: Missing (scope) annotation for callback without GDestroyNotify (valid: call, async)
+// EXPECT:21: Warning: Test: test_callback_invalid3: argument callback: Missing (scope) annotation for callback without GDestroyNotify (valid: call, async)