Split parameter and tag storage in annotationparser parse tree

This avoids tags overwriting parameters if they happen to
share the same name. For example, this was triggered by
valid code in libgnome-keyring.

tests/scanner/regress.c and tests/scanner/regress.h test
written by Colin Walters <walters@verbum.org>.

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

6 files changed, 90 insertions, 45 deletions

diff --git a/giscanner/annotationparser.py b/giscanner/annotationparser.py
index dd0c8fdb..16c75453 100644
--- a/giscanner/annotationparser.py
+++ b/giscanner/annotationparser.py
@@ -145,7 +145,7 @@ class DocBlock(object):
         self.value = None
         self.tags = odict()
         self.comment = None
-        self.params = []
+        self.params = odict()
         self.position = None
 
     def __cmp__(self, other):
@@ -158,9 +158,12 @@ class DocBlock(object):
         self.position = position
         self.options.position = position
 
-    def get(self, name):
+    def get_tag(self, name):
         return self.tags.get(name)
 
+    def get_param(self, name):
+        return self.params.get(name)
+
     def to_gtk_doc(self):
         options = ''
         if self.options:
@@ -170,19 +173,14 @@ class DocBlock(object):
         if 'SECTION' not in self.name:
             lines[0] += ':'
         lines[0] += options
-        tags = []
-        for name, tag in self.tags.iteritems():
-            if name in self.params:
-                lines.append(tag.to_gtk_doc_param())
-            else:
-                tags.append(tag)
-
+        for param in self.params.values():
+            lines.append(param.to_gtk_doc_param())
         lines.append('')
         for l in self.comment.split('\n'):
             lines.append(l)
-        if tags:
+        if self.tags:
             lines.append('')
-            for tag in tags:
+            for tag in self.tags.values():
                 lines.append(tag.to_gtk_doc_tag())
 
         comment = ''
@@ -197,6 +195,9 @@ class DocBlock(object):
         return comment
 
     def validate(self):
+        for param in self.params.values():
+            param.validate()
+
         for tag in self.tags.values():
             tag.validate()
 
@@ -720,7 +721,7 @@ class AnnotationParser(object):
                         message.warn("encountered multiple 'Returns' parameters or tags for "
                                      "'%s'." % (comment_block.name),
                                      position)
-                elif param_name in comment_block.tags.keys():
+                elif param_name in comment_block.params.keys():
                     column = result.start('parameter_name') + column_offset
                     marker = ' '*column + '^'
                     message.warn("multiple '@%s' parameters for identifier '%s':\n%s\n%s" %
@@ -732,9 +733,10 @@ class AnnotationParser(object):
                 tag.comment = param_description
                 if param_annotations:
                     tag.options = self.parse_options(tag, param_annotations)
-                comment_block.tags[param_name] = tag
-                if not param_name == TAG_RETURNS:
-                    comment_block.params.append(param_name)
+                if param_name == TAG_RETURNS:
+                    comment_block.tags[param_name] = tag
+                else:
+                    comment_block.params[param_name] = tag
                 current_param = tag
                 continue
 
@@ -785,7 +787,7 @@ class AnnotationParser(object):
                     comment_block.tags[TAG_RETURNS] = tag
                     current_tag = tag
                     continue
-                elif tag_name.lower() in _ALL_TAGS:
+                else:
                     if tag_name.lower() in comment_block.tags.keys():
                         column = result.start('tag_name') + column_offset
                         marker = ' '*column + '^'
@@ -858,20 +860,26 @@ class AnnotationParser(object):
             comment_block.comment = ''
 
         for tag in comment_block.tags.itervalues():
-            if tag.comment:
-                tag.comment = tag.comment.strip()
-            else:
-                tag.comment = None
+            self._clean_comment_block_part(tag)
 
-            if tag.value:
-                tag.value = tag.value.strip()
-            else:
-                tag.value = ''
+        for param in comment_block.params.itervalues():
+            self._clean_comment_block_part(param)
 
         # Validate and store block.
         comment_block.validate()
         return comment_block
 
+    def _clean_comment_block_part(self, part):
+        if part.comment:
+            part.comment = part.comment.strip()
+        else:
+            part.comment = None
+
+        if part.value:
+            part.value = part.value.strip()
+        else:
+            part.value = ''
+
     def _validate_multiline_annotation_continuation(self, line, original_line,
                                                           column_offset, position):
         '''
diff --git a/giscanner/introspectablepass.py b/giscanner/introspectablepass.py
index 77a0e4f7..97ccfe71 100644
--- a/giscanner/introspectablepass.py
+++ b/giscanner/introspectablepass.py
@@ -58,7 +58,7 @@ class IntrospectablePass(object):
         else:
             context = "return value: "
             if block:
-                return_tag = block.get(TAG_RETURNS)
+                return_tag = block.get_tag(TAG_RETURNS)
                 if return_tag:
                     position = return_tag.position
         message.warn_node(parent, prefix + context + text,
diff --git a/giscanner/maintransformer.py b/giscanner/maintransformer.py
index a71d6ef9..60c5bc43 100644
--- a/giscanner/maintransformer.py
+++ b/giscanner/maintransformer.py
@@ -24,7 +24,7 @@ from . import message
 from .annotationparser import (TAG_VFUNC, TAG_SINCE, TAG_DEPRECATED, TAG_RETURNS,
                                TAG_ATTRIBUTES, TAG_RENAME_TO, TAG_TYPE,
                                TAG_UNREF_FUNC, TAG_REF_FUNC, TAG_SET_VALUE_FUNC,
-                               TAG_GET_VALUE_FUNC, TAG_VALUE)
+                               TAG_GET_VALUE_FUNC, TAG_VALUE, TAG_TRANSFER)
 from .annotationparser import (OPT_ALLOW_NONE, OPT_ARRAY, OPT_ATTRIBUTE,
                                OPT_ELEMENT_TYPE, OPT_IN, OPT_INOUT,
                                OPT_INOUT_ALT, OPT_OUT, OPT_SCOPE,
@@ -141,7 +141,7 @@ usage is void (*_gtk_reserved1)(void);"""
     def _apply_annotation_rename_to(self, node, chain, block):
         if not block:
             return
-        rename_to = block.get(TAG_RENAME_TO)
+        rename_to = block.get_tag(TAG_RENAME_TO)
         if not rename_to:
             return
         rename_to = rename_to.value
@@ -231,13 +231,13 @@ usage is void (*_gtk_reserved1)(void);"""
         if isinstance(node, ast.Class):
             block = self._get_block(node)
             if block:
-                tag = block.get(TAG_UNREF_FUNC)
+                tag = block.get_tag(TAG_UNREF_FUNC)
                 node.unref_func = tag.value if tag else None
-                tag = block.get(TAG_REF_FUNC)
+                tag = block.get_tag(TAG_REF_FUNC)
                 node.ref_func = tag.value if tag else None
-                tag = block.get(TAG_SET_VALUE_FUNC)
+                tag = block.get_tag(TAG_SET_VALUE_FUNC)
                 node.set_value_func = tag.value if tag else None
-                tag = block.get(TAG_GET_VALUE_FUNC)
+                tag = block.get_tag(TAG_GET_VALUE_FUNC)
                 node.get_value_func = tag.value if tag else None
         if isinstance(node, ast.Constant):
             self._apply_annotations_constant(node)
@@ -319,7 +319,7 @@ usage is void (*_gtk_reserved1)(void);"""
         block = self._blocks.get(func.symbol)
         if block:
             if isinstance(param, ast.Parameter):
-                tag = block.tags.get(param.argname)
+                tag = block.params.get(param.argname)
             elif isinstance(param, ast.Return):
                 tag = block.tags.get(TAG_RETURNS)
             else:
@@ -599,11 +599,11 @@ usage is void (*_gtk_reserved1)(void);"""
 
         node.doc = block.comment
 
-        since_tag = block.get(TAG_SINCE)
+        since_tag = block.get_tag(TAG_SINCE)
         if since_tag is not None:
             node.version = since_tag.value
 
-        deprecated_tag = block.get(TAG_DEPRECATED)
+        deprecated_tag = block.get_tag(TAG_DEPRECATED)
         if deprecated_tag is not None:
             value = deprecated_tag.value
             if ': ' in value:
@@ -617,7 +617,7 @@ usage is void (*_gtk_reserved1)(void);"""
             if version is not None:
                 node.deprecated_version = version
 
-        annos_tag = block.get(TAG_ATTRIBUTES)
+        annos_tag = block.get_tag(TAG_ATTRIBUTES)
         if annos_tag is not None:
             for key, value in annos_tag.options.iteritems():
                 if value:
@@ -679,7 +679,7 @@ usage is void (*_gtk_reserved1)(void);"""
 
     def _apply_annotations_return(self, parent, return_, block):
         if block:
-            tag = block.get(TAG_RETURNS)
+            tag = block.get_tag(TAG_RETURNS)
         else:
             tag = None
         self._apply_annotations_param_ret_common(parent, return_, tag)
@@ -690,7 +690,7 @@ usage is void (*_gtk_reserved1)(void);"""
             declparams.add(parent.instance_parameter.argname)
         for param in params:
             if block:
-                tag = block.get(param.argname)
+                tag = block.get_param(param.argname)
             else:
                 tag = None
             self._apply_annotations_param(parent, param, tag)
@@ -716,7 +716,7 @@ usage is void (*_gtk_reserved1)(void);"""
                 text = ', should be one of %s' % (
                 ', '.join(repr(p) for p in unused), )
 
-            tag = block.get(doc_name)
+            tag = block.get_param(doc_name)
             message.warn(
                 '%s: unknown parameter %r in documentation comment%s' % (
                 block.name, doc_name, text),
@@ -744,7 +744,7 @@ usage is void (*_gtk_reserved1)(void);"""
     def _apply_annotations_field(self, parent, block, field):
         if not block:
             return
-        tag = block.get(field.name)
+        tag = block.get_param(field.name)
         if not tag:
             return
         t = tag.options.get(OPT_TYPE)
@@ -762,7 +762,7 @@ usage is void (*_gtk_reserved1)(void);"""
         self._apply_annotations_annotated(prop, block)
         if not block:
             return
-        transfer_tag = block.get(OPT_TRANSFER)
+        transfer_tag = block.get_tag(TAG_TRANSFER)
         if transfer_tag is not None:
             transfer = transfer_tag.value
             if transfer == OPT_TRANSFER_FLOATING:
@@ -770,7 +770,7 @@ usage is void (*_gtk_reserved1)(void);"""
             prop.transfer = transfer
         else:
             prop.transfer = self._get_transfer_default(parent, prop)
-        type_tag = block.get(TAG_TYPE)
+        type_tag = block.get_tag(TAG_TYPE)
         if type_tag:
             prop.type = self._resolve_toplevel(type_tag.value, prop.type, prop, parent)
 
@@ -781,8 +781,8 @@ usage is void (*_gtk_reserved1)(void);"""
         # We're only attempting to name the signal parameters if
         # the number of parameter tags (@foo) is the same or greater
         # than the number of signal parameters
-        if block and len(block.tags) > len(signal.parameters):
-            names = block.tags.items()
+        if block and len(block.params) > len(signal.parameters):
+            names = block.params.items()
             # Resolve real parameter names early, so that in later
             # phase we can refer to them while resolving annotations.
             for i, param in enumerate(signal.parameters):
@@ -806,7 +806,7 @@ usage is void (*_gtk_reserved1)(void);"""
         block = self._blocks.get(node.ctype)
         if not block:
             return
-        tag = block.get(TAG_VALUE)
+        tag = block.get_tag(TAG_VALUE)
         if tag:
             node.value = tag.value
 
@@ -824,7 +824,7 @@ usage is void (*_gtk_reserved1)(void);"""
         parent = chain[-1] if chain else None
         if not (block and parent):
             return
-        virtual = block.get(TAG_VFUNC)
+        virtual = block.get_tag(TAG_VFUNC)
         if not virtual:
             return
         invoker_name = virtual.value
diff --git a/tests/scanner/Regress-1.0-expected.gir b/tests/scanner/Regress-1.0-expected.gir
index 5ca8a577..f88fd9e4 100644
--- a/tests/scanner/Regress-1.0-expected.gir
+++ b/tests/scanner/Regress-1.0-expected.gir
@@ -1528,6 +1528,26 @@ Use with regress_test_obj_emit_sig_with_obj</doc>
         </parameter>
       </parameters>
     </function>
+    <function name="has_parameter_named_attrs"
+              c:identifier="regress_has_parameter_named_attrs">
+      <doc xml:whitespace="preserve">This test case mirrors GnomeKeyringPasswordSchema from
+libgnome-keyring.</doc>
+      <return-value transfer-ownership="none">
+        <type name="none" c:type="void"/>
+      </return-value>
+      <parameters>
+        <parameter name="foo" transfer-ownership="none">
+          <doc xml:whitespace="preserve">some int</doc>
+          <type name="gint" c:type="int"/>
+        </parameter>
+        <parameter name="attributes" transfer-ownership="none">
+          <doc xml:whitespace="preserve">list of attributes</doc>
+          <array zero-terminated="0" c:type="gpointer" fixed-size="32">
+            <type name="guint32" c:type="gpointer"/>
+          </array>
+        </parameter>
+      </parameters>
+    </function>
     <function name="introspectable_via_alias"
               c:identifier="regress_introspectable_via_alias">
       <return-value transfer-ownership="none">
diff --git a/tests/scanner/regress.c b/tests/scanner/regress.c
index 7238ddf8..544f895c 100644
--- a/tests/scanner/regress.c
+++ b/tests/scanner/regress.c
@@ -3610,3 +3610,17 @@ regress_test_struct_fixed_array_frob (RegressTestStructFixedArray *str)
   for (i = 0; i < G_N_ELEMENTS(str->array); i++)
     str->array[i] = 42 + i;
 }
+
+/**
+ * regress_has_parameter_named_attrs:
+ * @foo: some int
+ * @attributes: (type guint32) (array fixed-size=32): list of attributes
+ *
+ * This test case mirrors GnomeKeyringPasswordSchema from
+ * libgnome-keyring.
+ */
+void
+regress_has_parameter_named_attrs (int        foo,
+                                   gpointer   attributes)
+{
+}
diff --git a/tests/scanner/regress.h b/tests/scanner/regress.h
index 4138568d..1740145f 100644
--- a/tests/scanner/regress.h
+++ b/tests/scanner/regress.h
@@ -783,4 +783,7 @@ void regress_test_struct_fixed_array_frob (RegressTestStructFixedArray *str);
 	"POWERSHARE,PRODIGY,TLX,X400,GIF,CGM,WMF,BMP,MET,PMB,DIB,PICT,TIFF," \
 	"PDF,PS,JPEG,QTIME,MPEG,MPEG2,AVI,WAVE,AIFF,PCM,X509,PGP"
 
+void regress_has_parameter_named_attrs (int        foo,
+                                        gpointer   attributes);
+
 #endif /* __GITESTTYPES_H__ */