Detect non-ambiguous dangling XPath operators

pyxform/errors.py

@@ -187,6 +187,13 @@ class ErrorCode(Enum):
             "questions using these types and the entity list name."
         ),
     )
+    EXPRESSION_001 = Detail(
+        name="Expression - dangling operator",
+        msg=(
+            "[row : {row}] On the '{sheet}' sheet, the '{column}' value is invalid. "
+            "An operator must be followed by a value or expression."
+        ),
+    )
     HEADER_001: Detail = Detail(
         name="Headers - invalid missing header row",
         msg=(

pyxform/parsing/expression.py

@@ -117,6 +117,51 @@ def parse_expression(text: str) -> tuple[Token, ...]:
     return tuple(_EXPRESSION_LEXER.lex(text))
 
 
+_OPERAND_END_TOKEN_TYPES = {
+    "CLOSE_PAREN",
+    "DATE",
+    "DATETIME",
+    "NAME",
+    "NUMBER",
+    "PARENT_REF",
+    "PYXFORM_REF",
+    "PYXFORM_REF_END",
+    "SELF_REF",
+    "SYSTEM_LITERAL",
+    "TIME",
+    "XPATH_PRED_END",
+}
+_WORD_OPERATORS = {"and", "div", "mod", "or"}
+
+
+def _can_end_operand(token: Token) -> bool:
+    """Return whether a token can end the left operand of a word operator."""
+    return token.type in _OPERAND_END_TOKEN_TYPES or (
+        token.type == "OPS_MATH" and token.value == "*"
+    )
+
+
+def ends_with_dangling_operator(text: str) -> bool:
+    """Return whether an expression ends with an operator requiring an operand."""
+    tokens = tuple(
+        token for token in parse_expression(text) if token.type != "WHITESPACE"
+    )
+    if not tokens:
+        return False
+
+    last_token = tokens[-1]
+    token_value = str(last_token)
+    operator = token_value.strip()
+    if operator in _WORD_OPERATORS:
+        # Operator words can also be XPath node names. They are operators only when
+        # preceded by something that can end a left operand. A path separator, for
+        # example, means the word is a node name such as the `and` in `/data/and`.
+        return len(tokens) > 1 and _can_end_operand(token=tokens[-2])
+    if last_token.type in {"OPS_COMP", "OPS_UNION"}:
+        return True
+    return last_token.type == "OPS_MATH" and operator != "*"
+
+
 def is_xml_tag(value: str) -> bool:
     """Check if the input string contains only a valid XML tag / element name."""
     return value and bool(RE_NCNAME_NAMESPACED.fullmatch(value))

pyxform/validators/pyxform/expression.py

@@ -0,0 +1,101 @@
+"""Targeted validation for expressions in XLSForm workbook cells."""
+
+from collections.abc import Sequence
+from itertools import islice
+from typing import Any
+
+from pyxform import aliases, constants
+from pyxform.errors import ErrorCode, PyXFormError
+from pyxform.parsing.expression import ends_with_dangling_operator
+from pyxform.utils import default_is_dynamic
+
+ExpressionPath = tuple[str, ...]
+
+_SURVEY_EXPRESSION_PATHS = {
+    (constants.BIND, "relevant"),
+    (constants.BIND, "constraint"),
+    (constants.BIND, "calculate"),
+    (constants.BIND, "required"),
+    (constants.BIND, "readonly"),
+    (constants.CHOICE_FILTER,),
+    (constants.CONTROL, "jr:count"),
+    ("default",),
+}
+_SETTINGS_EXPRESSION_PATHS = {("instance_name",)}
+_ENTITIES_EXPRESSION_PATHS = {
+    (constants.EntityColumns.ENTITY_ID.value,),
+    (constants.EntityColumns.CREATE_IF.value,),
+    (constants.EntityColumns.UPDATE_IF.value,),
+    (constants.EntityColumns.LABEL.value,),
+}
+_EXPRESSION_PATHS_BY_SHEET = {
+    constants.SURVEY: _SURVEY_EXPRESSION_PATHS,
+    constants.SETTINGS: _SETTINGS_EXPRESSION_PATHS,
+    constants.ENTITIES: _ENTITIES_EXPRESSION_PATHS,
+}
+
+
+def _get_source_headers(
+    sheet_data: Sequence[dict[str, Any]],
+    sheet_header: Sequence[dict[str, Any]] | None,
+) -> tuple[str, ...]:
+    """Get original headers in the same order used by header normalization."""
+    if sheet_header:
+        return tuple(sheet_header[0])
+
+    headers: dict[str, None] = {}
+    for row in islice(sheet_data, 0, 100):
+        for header in row:
+            headers[header] = None
+    return tuple(headers)
+
+
+def _get_value(row: dict[str, Any], path: ExpressionPath) -> Any:
+    """Get a possibly nested value from a normalized workbook row."""
+    value: Any = row
+    for token in path:
+        if not isinstance(value, dict):
+            return None
+        value = value.get(token)
+    return value
+
+
+def validate_dangling_operators(
+    sheet_name: str,
+    source_sheet_data: Sequence[dict[str, Any]],
+    source_sheet_header: Sequence[dict[str, Any]] | None,
+    normalized_sheet_data: Sequence[dict[str, Any]],
+    normalized_headers: tuple[ExpressionPath, ...],
+) -> None:
+    """Reject recognized expressions ending with an operator needing an operand."""
+    expression_paths = _EXPRESSION_PATHS_BY_SHEET[sheet_name]
+    source_headers = _get_source_headers(
+        sheet_data=source_sheet_data, sheet_header=source_sheet_header
+    )
+    expression_columns = tuple(
+        (path, source_header)
+        for path, source_header in zip(normalized_headers, source_headers, strict=False)
+        if path in expression_paths
+    )
+
+    for row_number, row in enumerate(normalized_sheet_data, start=2):
+        if sheet_name == constants.SURVEY and aliases.yes_no.get(row.get("disabled")):
+            continue
+
+        for path, source_header in expression_columns:
+            value = _get_value(row=row, path=path)
+            if not isinstance(value, str) or not value:
+                continue
+            if path == ("default",) and not default_is_dynamic(
+                element_default=value, element_type=row.get(constants.TYPE)
+            ):
+                continue
+            if ends_with_dangling_operator(text=value):
+                raise PyXFormError(
+                    code=ErrorCode.EXPRESSION_001,
+                    context={
+                        "row": row_number,
+                        "sheet": sheet_name,
+                        "column": source_header,
+                    },
+                )

pyxform/xls2json.py

@@ -39,6 +39,7 @@
 from pyxform.validators.pyxform import settings as validate_settings
 from pyxform.validators.pyxform.android_package_name import validate_android_package_name
 from pyxform.validators.pyxform.choices import validate_and_clean_choices
+from pyxform.validators.pyxform.expression import validate_dangling_operators
 from pyxform.validators.pyxform.pyxform_reference import (
     has_pyxform_reference,
     is_pyxform_reference,
@@ -274,6 +275,13 @@ def workbook_to_json(
             header_aliases=aliases.settings_header,
             header_columns=set(Survey.get_slot_names()),
         )
+        validate_dangling_operators(
+            sheet_name=constants.SETTINGS,
+            source_sheet_data=workbook_dict.settings,
+            source_sheet_header=settings_sheet_headers,
+            normalized_sheet_data=settings_sheet.data,
+            normalized_headers=settings_sheet.headers,
+        )
         settings = settings_sheet.data[0]
         validate_settings.validate_name(name=settings.get(constants.NAME, None))
     else:
@@ -379,6 +387,13 @@ def workbook_to_json(
             header_aliases=aliases.entities_header,
             header_columns={i.value for i in constants.EntityColumns.value_list()},
         )
+        validate_dangling_operators(
+            sheet_name=constants.ENTITIES,
+            source_sheet_data=workbook_dict.entities,
+            source_sheet_header=workbook_dict.entities_header,
+            normalized_sheet_data=entities_sheet.data,
+            normalized_headers=entities_sheet.headers,
+        )
         entity_declarations = get_entity_declarations(entities_sheet=entities_sheet.data)
         entity_variable_references = get_entity_variable_references(
             entity_declarations=entity_declarations
@@ -403,6 +418,13 @@ def workbook_to_json(
         strip_whitespace=clean_text_values_enabled,
     )
     survey_sheet.data = dealias_types(dict_array=survey_sheet.data)
+    validate_dangling_operators(
+        sheet_name=constants.SURVEY,
+        source_sheet_data=workbook_dict.survey,
+        source_sheet_header=workbook_dict.survey_header,
+        normalized_sheet_data=survey_sheet.data,
+        normalized_headers=survey_sheet.headers,
+    )
 
     # Check for missing translations. The choices sheet is checked here so that the
     # warning can be combined into one message.

tests/parsing/test_expression.py

@@ -1,6 +1,10 @@
 from enum import Enum
 
-from pyxform.parsing.expression import is_xml_tag, parse_expression
+from pyxform.parsing.expression import (
+    ends_with_dangling_operator,
+    is_xml_tag,
+    parse_expression,
+)
 
 from tests.fixtures.lexer_cases import LexerCases
 from tests.pyxform_test_case import PyxformTestCase
@@ -410,3 +414,74 @@ def test_parse_expression(self):
                 self.assertEqual(
                     token_types, tuple(t.type for t in parse_expression(text=case))
                 )
+
+    def test_ends_with_dangling_operator(self):
+        """Should identify supported operators with varied trailing whitespace."""
+        operators = (
+            "=",
+            "!=",
+            "<",
+            ">",
+            "<=",
+            ">=",
+            "+",
+            "-",
+            "div",
+            "mod",
+            "and",
+            "or",
+            "|",
+        )
+        for operator in operators:
+            for whitespace in ("", "   ", "\t \n"):
+                expression = f"${{q1}} {operator}{whitespace}"
+                with self.subTest(expression=expression):
+                    self.assertTrue(ends_with_dangling_operator(expression))
+
+        for expression in ("${q1}and", "true()and", "/data/* and"):
+            with self.subTest(expression=expression):
+                self.assertTrue(ends_with_dangling_operator(expression))
+
+    def test_does_not_end_with_dangling_operator(self):
+        """Should accept complete expressions and quoted operator characters."""
+        for expression in (
+            "${q1} = 1",
+            "${q1} != ''",
+            "${q1} < ${q2}",
+            "${q1} > 0",
+            "${q1} <= 5",
+            "${q1} >= 5",
+            "${q1} + 1",
+            "${q1} - 1",
+            "${q1} div 2",
+            "${q1} mod 2",
+            "${q1} and ${q2}",
+            "${q1} or ${q2}",
+            "${q1} | ${q2}",
+            "/data/*",
+            "/data/and",
+            "/data/or",
+            "/data/div",
+            "/data/mod",
+        ):
+            with self.subTest(expression=expression):
+                self.assertFalse(ends_with_dangling_operator(expression))
+
+        for operator in (
+            "=",
+            "!=",
+            "<",
+            ">",
+            "<=",
+            ">=",
+            "+",
+            "-",
+            "div",
+            "mod",
+            "and",
+            "or",
+            "|",
+        ):
+            expression = f"'{operator}'"
+            with self.subTest(expression=expression):
+                self.assertFalse(ends_with_dangling_operator(expression))