Merge pull request #674 from lindsay-stevens/pyxform-576

576: fix _count suffix name clash with repeats targeting another item

pyxform/parsing/expression.py

@@ -0,0 +1,14 @@
+from typing import Iterable
+
+from pyxform.utils import parse_expression
+
+
+def is_single_token_expression(expression: str, token_types: Iterable[str]) -> bool:
+    """
+    Does the expression contain single token of one of the provided token types?
+    """
+    tokens, _ = parse_expression(text=expression.strip())
+    if 1 == len(tokens) and tokens[0].name in token_types:
+        return True
+    else:
+        return False

pyxform/xls2json.py

@@ -22,6 +22,7 @@
     validate_entity_saveto,
 )
 from pyxform.errors import PyXFormError
+from pyxform.parsing.expression import is_single_token_expression
 from pyxform.utils import PYXFORM_REFERENCE_REGEX, default_is_dynamic
 from pyxform.validators.pyxform import parameters_generic, select_from_file
 from pyxform.validators.pyxform.android_package_name import validate_android_package_name
@@ -1017,24 +1018,29 @@ def workbook_to_json(
                         )
                     new_json_dict[constants.COLUMNS] = choices[list_name]
 
-                # Generate a new node for the jr:count column so
-                # xpath expressions can be used.
+                # Generate a new node for the jr:count column so xpath expressions can be used.
                 repeat_count_expression = new_json_dict.get("control", {}).get("jr:count")
                 if repeat_count_expression:
-                    generated_node_name = new_json_dict["name"] + "_count"
-                    parent_children_array.append(
-                        {
-                            "name": generated_node_name,
-                            "bind": {
-                                "readonly": "true()",
-                                "calculate": repeat_count_expression,
-                            },
-                            "type": "calculate",
-                        }
-                    )
-                    new_json_dict["control"]["jr:count"] = (
-                        "${" + generated_node_name + "}"
+                    # Simple expressions don't require a new node, they can reference directly.
+                    simple_expression = is_single_token_expression(
+                        expression=repeat_count_expression, token_types=["PYXFORM_REF"]
                     )
+                    if not simple_expression:
+                        generated_node_name = new_json_dict["name"] + "_count"
+                        parent_children_array.append(
+                            {
+                                "name": generated_node_name,
+                                "bind": {
+                                    "readonly": "true()",
+                                    "calculate": repeat_count_expression,
+                                },
+                                "type": "calculate",
+                            }
+                        )
+                        # This re-directs the body/repeat ref to the above generated node.
+                        new_json_dict["control"]["jr:count"] = (
+                            "${" + generated_node_name + "}"
+                        )
 
                 # Code to deal with table_list appearance flags
                 # (for groups of selects)

tests/test_expected_output/repeat_date_test.xml

@@ -1,13 +1,12 @@
 <?xml version="1.0"?>
-<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:ev="http://www.w3.org/2001/xml-events" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:jr="http://openrosa.org/javarosa" xmlns:orx="http://openrosa.org/xforms" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:odk="http://www.opendatakit.org/xforms">
+<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:ev="http://www.w3.org/2001/xml-events" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:jr="http://openrosa.org/javarosa" xmlns:odk="http://www.opendatakit.org/xforms" xmlns:orx="http://openrosa.org/xforms" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
   <h:head>
     <h:title>repeat_date_test</h:title>
     <model odk:xforms-version="1.0.0">
       <instance>
         <repeat_date_test id="repeat_date_test">
           <generated_note_name_2/>
           <repeat_count/>
-          <repeat_test_count/>
           <repeat_test jr:template="">
             <table_list_3/>
             <table_list_4/>
@@ -36,7 +35,6 @@
       </instance>
       <bind nodeset="/repeat_date_test/generated_note_name_2" readonly="true()" type="string"/>
       <bind calculate="1" nodeset="/repeat_date_test/repeat_count" type="string"/>
-      <bind calculate=" /repeat_date_test/repeat_count " nodeset="/repeat_date_test/repeat_test_count" readonly="true()" type="string"/>
       <bind nodeset="/repeat_date_test/repeat_test/table_list_3" type="string"/>
       <bind nodeset="/repeat_date_test/repeat_test/table_list_4" type="string"/>
       <bind nodeset="/repeat_date_test/generated_note_name_8" readonly="true()" type="string"/>
@@ -49,7 +47,7 @@
     </input>
     <group ref="/repeat_date_test/repeat_test">
       <label></label>
-      <repeat jr:count=" /repeat_date_test/repeat_test_count " nodeset="/repeat_date_test/repeat_test">
+      <repeat jr:count=" /repeat_date_test/repeat_count " nodeset="/repeat_date_test/repeat_test">
         <select1 ref="/repeat_date_test/repeat_test/table_list_3">
           <label>Q1</label>
           <itemset nodeset="instance('yes_no')/root/item">

tests/test_repeat.py

@@ -984,3 +984,44 @@ def test_repeat_with_reference_path_after_instance_not_in_predicate_not_using_cu
                 """<bind calculate="concat(instance('item')/root/item[index = current()/../pos5 ]/label,  ../pos5  + 1)" nodeset="/data/rep5/item5" type="string"/>""",  # noqa pylint: disable=line-too-long
             ],
         )
+
+    def test_repeat_count_item_with_same_suffix_as_repeat_is_ok(self):
+        """Should not have a name clash, the referenced item should be used directly."""
+        md = """
+        | survey |              |         |       |              |
+        |        | type         | name    | label | repeat_count |
+        |        | integer      | a_count | 1     |              |
+        |        | begin repeat | a       | 2     | ${a_count}   |
+        |        | text         | b       | 3     |              |
+        |        | end repeat   | a       |       |              |
+        """
+        self.assertPyxformXform(
+            md=md,
+            debug=True,
+            xml__xpath_match=[
+                # repeat references existing count element directly.
+                """
+                /h:html/h:body/x:group[@ref='/test_name/a']/x:repeat[
+                  @jr:count=' /test_name/a_count '
+                  and @nodeset='/test_name/a'
+                  and ./x:input[@ref='/test_name/a/b']
+                ]
+                """,
+                # binding for existing count element but no calculate binding.
+                """
+                /h:html/h:head/x:model[
+                  ./x:bind[@nodeset='/test_name/a_count' and @type='int']
+                  and not(./x:bind[
+                    @calculate=' /test_name/a_count'
+                    and @nodeset='/test_name/a_count'
+                    and @readonly='true()'
+                  ])
+                ]
+                """,
+                # no duplicated element in the instance.
+                """
+                /h:html/h:head/x:model/x:instance/x:test_name/x:a_count
+                """,
+            ],
+            run_odk_validate=True,
+        )