From c4ba1bd2565a7340dcba52354d2e7c763b9826aa Mon Sep 17 00:00:00 2001 From: lindsay stevens Date: Tue, 9 Dec 2025 01:21:29 +1100 Subject: [PATCH 1/8] chg: remove redundant variable --- pyxform/question.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pyxform/question.py b/pyxform/question.py index 40603d37..ccdbdb08 100644 --- a/pyxform/question.py +++ b/pyxform/question.py @@ -343,9 +343,8 @@ def get_options(self, choices: Iterable[dict]) -> Generator[Option, None, None]: requires_itext = True else: choice_label = option.label - label_is_dict = isinstance(choice_label, dict) # Multi-language: dict of labels etc per language. Can be just a string. - if label_is_dict: + if isinstance(choice_label, dict): requires_itext = True # Dynamic label: string contains a pyxform reference. elif choice_label and has_pyxform_reference(choice_label): From 3880b38abf946dba72745fd536e84848ac2bf068 Mon Sep 17 00:00:00 2001 From: lindsay stevens Date: Tue, 9 Dec 2025 01:22:46 +1100 Subject: [PATCH 2/8] add: type annotations --- pyxform/survey_element.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pyxform/survey_element.py b/pyxform/survey_element.py index 263f18d3..717d26e0 100644 --- a/pyxform/survey_element.py +++ b/pyxform/survey_element.py @@ -7,7 +7,7 @@ import warnings from collections.abc import Callable, Generator, Iterable, Mapping from itertools import chain -from typing import TYPE_CHECKING, Optional +from typing import TYPE_CHECKING, Any, Optional from pyxform import aliases as alias from pyxform import constants as const @@ -294,7 +294,7 @@ def _delete_keys_from_dict(self, dictionary: dict, keys: Iterable[str]): if isinstance(value, dict): self._delete_keys_from_dict(value, keys) - def copy(self): + def copy(self) -> dict[str, Any]: return {k: self[k] for k in self} def to_json_dict(self, delete_keys: Iterable[str] | None = None) -> dict: @@ -343,7 +343,7 @@ def to_json_dict(self, delete_keys: Iterable[str] | None = None) -> dict: return result - def to_json(self): + def to_json(self) -> str: return json.dumps(self.to_json_dict()) def json_dump(self, path=""): From a11cee8724adc1569b5ccc1d0994dc75b4b98c9b Mon Sep 17 00:00:00 2001 From: lindsay stevens Date: Tue, 9 Dec 2025 01:32:41 +1100 Subject: [PATCH 3/8] chg: simplify choices xpath test helper expression --- tests/xpath_helpers/choices.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/xpath_helpers/choices.py b/tests/xpath_helpers/choices.py index 967ba08b..b3990714 100644 --- a/tests/xpath_helpers/choices.py +++ b/tests/xpath_helpers/choices.py @@ -32,9 +32,11 @@ def model_instance_choices_itext(cname: str, choices: tuple[str, ...]): choices_xp = "\n and ".join( ( f""" - ./x:item/x:name/text() = '{cv}' - and not(./x:item/x:label) - and ./x:item/x:itextId = '{cname}-{idx}' + ./x:item[ + ./x:name/text() = '{cv}' + and not(./x:label) + and ./x:itextId = '{cname}-{idx}' + ] """ for idx, cv in enumerate(choices) ) From 15ff6879f38a272ef433c4701c0d3e21d8de8b51 Mon Sep 17 00:00:00 2001 From: lindsay stevens Date: Tue, 9 Dec 2025 02:05:39 +1100 Subject: [PATCH 4/8] chg: indicate which sheet a ${} replacement error comes from - this error is caught by survey.py normally but in that scope it is not possible to provide a user-friendly error message with the source of the problem i.e. sheet name, column name, and row number. - overall changes: - move string validation func `clean_text_values` from xls2json to sheet_headers, which (via dealias_and_group_headers) is already iterating over the sheet data (to avoid introducing an extra iteration over the sheet data) before the main round of processing in workbook_to_json. - move pyxform ref validation the end of `workbook_to_json` so that the (mostly final) list of question names can be used for the validation (excludes elements added for `loops` which is done in `builder.py` and refactoring that may be significant) - try to reduce the amount of re-parsing, or parsing for no reason, by adding some selectivity to in pyxform_reference.py. For example references don't work in external_choices at all, and aren't resolved in a useful way for extra (choice filter) columns (i.e. the reference goes into element text with no `output` wrapper). - added tests to the main test modules for each validated sheet. --- pyxform/entities/entities_parsing.py | 5 +- pyxform/errors.py | 13 +- pyxform/parsing/sheet_headers.py | 48 +++- .../validators/pyxform/pyxform_reference.py | 206 +++++++++++--- pyxform/validators/pyxform/question_types.py | 2 +- pyxform/xls2json.py | 82 ++---- tests/entities/test_update_survey.py | 22 ++ tests/test_choices_sheet.py | 266 +++++++++++++++++- tests/test_fields.py | 163 +++++++++++ tests/test_last_saved.py | 6 +- tests/test_settings.py | 47 ++++ tests/test_sheet_columns.py | 1 + .../pyxform/test_pyxform_reference.py | 34 ++- 13 files changed, 777 insertions(+), 118 deletions(-) diff --git a/pyxform/entities/entities_parsing.py b/pyxform/entities/entities_parsing.py index 3167911a..ccd7486d 100644 --- a/pyxform/entities/entities_parsing.py +++ b/pyxform/entities/entities_parsing.py @@ -260,11 +260,12 @@ def get_validated_repeat_name(entity) -> str | None: match = parse_pyxform_references(value=value, match_limit=1, match_full=True) except PyXFormError as e: e.context.update(sheet="entities", column="repeat", row=2) + raise else: - if not match or not is_xml_tag(match[0]): + if not match or match[0].last_saved: raise PyXFormError(ENTITY001.format(value=value)) else: - return match[0] + return match[0].name def validate_entity_saveto( diff --git a/pyxform/errors.py b/pyxform/errors.py index f744f12a..bc6b3eba 100644 --- a/pyxform/errors.py +++ b/pyxform/errors.py @@ -56,10 +56,19 @@ class ErrorCode(Enum): ), ) PYREF_003: Detail = Detail( - name="PyXForm Reference Question Not Found", + name="PyXForm Reference Name Not Found", msg=( "[row : {row}] On the '{sheet}' sheet, the '{column}' value is invalid. " - "Reference variables must refer to a question name. Could not find '{q}'." + "Reference variables must contain a name from the 'survey' sheet. Could not " + "find the name '{q}'." + ), + ) + PYREF_004: Detail = Detail( + name="PyXForm Reference Duplicate Name", + msg=( + "[row : {row}] On the '{sheet}' sheet, the '{column}' value is invalid. " + "Reference variables names must be unique anywhere in the 'survey'. The name " + "'{q}' appears more than once." ), ) INTERNAL_001: Detail = Detail( diff --git a/pyxform/parsing/sheet_headers.py b/pyxform/parsing/sheet_headers.py index 7337e5da..aa23b1d3 100644 --- a/pyxform/parsing/sheet_headers.py +++ b/pyxform/parsing/sheet_headers.py @@ -1,10 +1,15 @@ +import re from collections.abc import Container, Sequence from itertools import chain, islice from typing import Any from pyxform import constants from pyxform.errors import PyXFormError +from pyxform.parsing.expression import maybe_strip +from pyxform.xls2json_backends import RE_WHITESPACE +SMART_QUOTES = {"\u2018": "'", "\u2019": "'", "\u201c": '"', "\u201d": '"'} +RE_SMART_QUOTES = re.compile(r"|".join(re.escape(old) for old in SMART_QUOTES)) INVALID_HEADER = ( "Invalid headers provided for sheet: '{sheet_name}'. For XLSForms, this may be due " "a missing header row, in which case add a header row as per the reference template " @@ -24,6 +29,25 @@ ) +def clean_text_values( + value: str, + strip_whitespace: bool = False, +) -> str: + """ + Replace "smart" quotes with regular quotes and optionally collapse whitespace characters. + + :param value: The string to process. + :param strip_whitespace: If True, collapse sequences of whitespace to a single space. + """ + if isinstance(value, str) and value: + # Remove extraneous whitespace characters. + if strip_whitespace: + value = RE_WHITESPACE.sub(" ", maybe_strip(value)) + # Replace "smart" quotes with regular quotes. + value = RE_SMART_QUOTES.sub(lambda m: SMART_QUOTES[m.group(0)], value) + return value + + def merge_dicts( dict_a: dict, dict_b: dict, default_key: str = constants.DEFAULT_LANGUAGE_VALUE ) -> dict: @@ -149,7 +173,10 @@ def process_row( sheet_name: str, row: dict[str, str], header_key: dict[str, tuple[str, ...]], + row_number: int, default_language: str = constants.DEFAULT_LANGUAGE_VALUE, + strip_whitespace: bool = False, + add_row_number: bool = False, ) -> dict[str, str]: """ Convert original headers and values to a possibly nested structure. @@ -157,15 +184,17 @@ def process_row( :param sheet_name: Name of the sheet data being processed. :param row: Original XLSForm data row. :param header_key: Mapping from original headers to headers split on a delimiter. + :param row_number: The row number from the input data. :param default_language: Default translation language for the form, used to group used to group labels/hints/etc without a language specified with localized versions. + :param strip_whitespace: If True, collapse sequences of whitespace to a single space + :param add_row_number: If True, add a "__row" key with the row number from the input data. """ out_row = {} for header, val in row.items(): + val = clean_text_values(value=val, strip_whitespace=strip_whitespace) tokens = header_key.get(header, None) - if header == "__row": - out_row[header] = val - elif not tokens: + if not tokens: raise PyXFormError( INVALID_HEADER.format(sheet_name=sheet_name, header=header) ) @@ -174,7 +203,8 @@ def process_row( else: new_value = list_to_nested_dict((*tokens[1:], val)) out_row = merge_dicts(out_row, {tokens[0]: new_value}, default_language) - + if add_row_number: + out_row["__row"] = row_number return out_row @@ -186,6 +216,8 @@ def dealias_and_group_headers( header_columns: set[str], headers_required: set[str] | None = None, default_language: str = constants.DEFAULT_LANGUAGE_VALUE, + strip_whitespace: bool = False, + add_row_number: bool = False, ) -> DealiasAndGroupHeadersResult: """ Normalise headers and group keys that contain a delimiter. @@ -205,6 +237,9 @@ def dealias_and_group_headers( :param headers_required: Required columns for the sheet. :param default_language: Default translation language for the form, used to group used to group labels/hints/etc without a language specified with localized versions. + :param strip_whitespace: If True, collapse sequences of whitespace to a single space + in the data rows. + :param add_row_number: If True, add a "__row" key with the row number from the input data. """ header_key: dict[str, tuple[str, ...]] = {} @@ -247,9 +282,12 @@ def dealias_and_group_headers( sheet_name=sheet_name, row=row, header_key=header_key, + row_number=row_number, default_language=default_language, + strip_whitespace=strip_whitespace, + add_row_number=add_row_number, ) - for row in sheet_data + for row_number, row in enumerate(sheet_data, start=2) ) if headers_required and (data or sheet_name == constants.SURVEY): missing = {h for h in headers_required if h not in {h[0] for h in tokens_key}} diff --git a/pyxform/validators/pyxform/pyxform_reference.py b/pyxform/validators/pyxform/pyxform_reference.py index 855c297a..dc474e39 100644 --- a/pyxform/validators/pyxform/pyxform_reference.py +++ b/pyxform/validators/pyxform/pyxform_reference.py @@ -10,9 +10,12 @@ how likely is it that similar strings are close to each other vs. randomly dispersed? """ -from collections.abc import Generator +from collections import Counter +from collections.abc import Generator, Sequence from functools import lru_cache +from typing import TYPE_CHECKING +from pyxform import aliases from pyxform import constants as co from pyxform.errors import ErrorCode, PyXFormError from pyxform.parsing.expression import ( @@ -20,6 +23,17 @@ RE_PYXFORM_REF_OUTER, ) +if TYPE_CHECKING: + from pyxform.xls2json_backends import DefinitionData + + +class ParsedReference: + __slots__ = ("last_saved", "name") + + def __init__(self, name: str, last_saved: bool = False): + self.name: str = name + self.last_saved: bool = last_saved + def is_pyxform_reference_candidate(value: str) -> bool: """ @@ -38,7 +52,7 @@ def _parse( value: str, match_limit: int | None = None, match_full: bool = False, -) -> Generator[str, None, None]: +) -> Generator[ParsedReference, None, None]: """ Parse the string and return reference target(s) e.g. `name` from `${name}`. @@ -73,12 +87,19 @@ def _parse( ref_candidate = match.group("pyxform_ref") # Although it's an "any" match pattern, fullmatch is used to require "only". # Return the ref_candidate since it has original string start/end positions. - if ref_candidate and RE_PYXFORM_REF_INNER.fullmatch(ref_candidate): - if match_limit is not None and count >= match_limit: - raise PyXFormError(code=ErrorCode.PYREF_002) - - yield ref_candidate - count += 1 + if ref_candidate: + ref_inner = RE_PYXFORM_REF_INNER.fullmatch(ref_candidate) + if ref_inner: + if match_limit is not None and count >= match_limit: + raise PyXFormError(code=ErrorCode.PYREF_002) + + yield ParsedReference( + name=ref_inner.group("ncname"), + last_saved=ref_inner.group("last_saved") is not None, + ) + count += 1 + else: + raise PyXFormError(code=ErrorCode.PYREF_001) else: raise PyXFormError(code=ErrorCode.PYREF_001) @@ -120,9 +141,7 @@ def has_pyxform_reference_with_last_saved(value: str) -> bool: :param value: The string to inspect. """ try: - return len(value) > 14 and any( - i.startswith("last-saved#") for i in _parse(value=value) - ) + return len(value) > 14 and any(i.last_saved for i in _parse(value=value)) except (StopIteration, PyXFormError): return False @@ -132,7 +151,7 @@ def parse_pyxform_references( value: str, match_limit: int | None = None, match_full: bool = False, -) -> tuple[str, ...]: +) -> tuple[ParsedReference, ...]: """ Parse all pyxform references in a string. @@ -146,32 +165,147 @@ def parse_pyxform_references( def validate_pyxform_reference_syntax( - value: str, sheet_name: str, row_number: int, column: str -) -> tuple[str, ...] | None: + sheet_name: str, + sheet_data: Sequence[dict[str, str]], + element_names: Counter, + limit_to_columns: set[str] | None = None, + ignore_columns: set[str] | None = None, +) -> None: """ - Parse all pyxform references in a string, and raise an error if any are malformed. + Parse all pyxform references, and raise an error if any are malformed or invalid. - Generally the same as `parse_pyxform_references` but adds the XLSForm context to the - error message, if any. - - :param value: The string to inspect. :param sheet_name: The XLSForm sheet the value is from. - :param row_number: The XLSForm row the value is from. - :param column: The XLSForm column the value is from. + :param limit_to_columns: Only parse values in these columns. + :param ignore_columns: Do not parse values in these columns. + :param sheet_data: The XLSForm sheet data. + :param element_names: The names in the 'survey' sheet 'name' column. """ - # Skip columns in potentially large sheets where references are not allowed. - if sheet_name == co.SURVEY: - if column in {co.TYPE, co.NAME}: - return None - elif sheet_name == co.CHOICES: - if column in {co.LIST_NAME_S, co.LIST_NAME_U, co.NAME}: - return None - elif sheet_name == co.ENTITIES: - if column in {co.LIST_NAME_S, co.LIST_NAME_U}: - return None + if not sheet_data: + return + + for row_number, row in enumerate(sheet_data, start=2): + for column, value in row.items(): + if limit_to_columns and column not in limit_to_columns: + continue + if ignore_columns and column in ignore_columns: + continue + try: + refs = parse_pyxform_references(value=value) + except PyXFormError as e: + e.context.update(sheet=sheet_name, column=column, row=row_number) + raise + if refs: + for ref in refs: + element_count = element_names.get(ref.name, None) + if element_count is None: + raise PyXFormError( + code=ErrorCode.PYREF_003, + context={ + "row": row_number, + "sheet": sheet_name, + "column": column, + "q": ref.name, + }, + ) + elif 1 != element_count: + raise PyXFormError( + code=ErrorCode.PYREF_004, + context={ + "row": row_number, + "sheet": sheet_name, + "column": column, + "q": ref.name, + }, + ) + + +def validate_pyxform_references_in_workbook( + workbook_dict: "DefinitionData", + survey_headers: tuple[tuple[str, ...], ...], + choices_headers: tuple[tuple[str, ...], ...], + element_names: Counter, +) -> None: + """ + Parse pyxform references, and raise an error if any are malformed or invalid. - try: - return parse_pyxform_references(value=value) - except PyXFormError as e: - e.context.update(sheet=sheet_name, column=column, row=row_number) - raise + The original workbook_dict data is used to a) allow row/column references in error + message and b) avoid needing to iterate into sheet-specific nested data structures. + + The external_choices sheet isn't checked at all since this data is written to CSV for + verbatim lookups and so cannot contain dynamic references. + + In the survey sheet, references aren't allowed for the 'type' and 'name' columns and + this is validated separately, but since these columns have aliases, the parsed + survey_headers is required to get the relevant actual column names to ignore. + + In the choices sheet, references are only inserted for translatable columns, and the + choices sheet can contain a lot of extra data (e.g. for choice filters). So the + parsed/validated choices_header is used to ignore extra data, using a positional + lookup e.g. if 'label' is column 3 -> get 3rd column 'label::en'. + + :param workbook_dict: The XLSForm data. + :param survey_headers: The parsed column headers for the survey sheet. + :param choices_headers: The parsed column headers for the choices sheet. + :param element_names: The names in the 'survey' sheet 'name' column. + """ + # In order of likely smallest to largest. + validate_pyxform_reference_syntax( + sheet_name=co.SETTINGS, + sheet_data=workbook_dict.settings, + element_names=element_names, + ) + # Avoids circular import. + from pyxform.entities.entities_parsing import EC + + validate_pyxform_reference_syntax( + sheet_name=co.ENTITIES, + sheet_data=workbook_dict.entities, + element_names=element_names, + ignore_columns={co.LIST_NAME_S, co.LIST_NAME_U, EC.REPEAT}, + ) + + # type is validated against the question_type_dict. + # name is validated against XML tag name rules. + # trigger is validated against question names only (not groups or repeats). + survey_ignore_columns = {co.TYPE, co.NAME, "trigger"} + if workbook_dict.survey_header: + survey_ignore_columns = { + tuple(workbook_dict.survey_header[0])[i] + for i, c in enumerate(survey_headers, start=0) + if c[0] in survey_ignore_columns + } + else: + # The 'survey_header' may not be populated if pyxform is used as a library and + # the caller passes in incomplete dict data (e.g. not using a pyxform parser). + survey_ignore_columns = { + tuple(workbook_dict.survey[0])[i] + for i, c in enumerate(survey_headers, start=0) + if c[0] in survey_ignore_columns + } + validate_pyxform_reference_syntax( + sheet_name=co.SURVEY, + sheet_data=workbook_dict.survey, + element_names=element_names, + ignore_columns=survey_ignore_columns, + ) + + if workbook_dict.choices_header: + choices_limit_to_columns = { + tuple(workbook_dict.choices_header[0])[i] + for i, c in enumerate(choices_headers, start=0) + if c[0] in aliases.TRANSLATABLE_CHOICES_COLUMNS + } + else: + # The 'choices_header' may not be populated if pyxform is used as a library and + # the caller passes in incomplete dict data (e.g. not using a pyxform parser). + choices_limit_to_columns = { + tuple(workbook_dict.choices[0])[i] + for i, c in enumerate(choices_headers, start=0) + if c[0] in aliases.TRANSLATABLE_CHOICES_COLUMNS + } + validate_pyxform_reference_syntax( + sheet_name=co.CHOICES, + sheet_data=workbook_dict.choices, + element_names=element_names, + limit_to_columns=choices_limit_to_columns, + ) diff --git a/pyxform/validators/pyxform/question_types.py b/pyxform/validators/pyxform/question_types.py index 8cd25e83..8e24eca4 100644 --- a/pyxform/validators/pyxform/question_types.py +++ b/pyxform/validators/pyxform/question_types.py @@ -77,7 +77,7 @@ def process_trigger( try: trigger = tuple( - r + r.name for t in trigger.split(",") for r in parse_pyxform_references(value=t, match_limit=1) ) diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py index 6500c48a..53a981f8 100644 --- a/pyxform/xls2json.py +++ b/pyxform/xls2json.py @@ -5,7 +5,7 @@ import os import re import sys -from collections.abc import Sequence +from collections import Counter from typing import IO, Any from pyxform import aliases, constants @@ -22,10 +22,7 @@ validate_entity_saveto, ) from pyxform.errors import Detail, PyXFormError -from pyxform.parsing.expression import ( - is_xml_tag, - maybe_strip, -) +from pyxform.parsing.expression import is_xml_tag from pyxform.parsing.sheet_headers import dealias_and_group_headers from pyxform.question_type_dictionary import get_meta_group from pyxform.utils import ( @@ -40,18 +37,12 @@ from pyxform.validators.pyxform.pyxform_reference import ( has_pyxform_reference, is_pyxform_reference, - validate_pyxform_reference_syntax, + validate_pyxform_references_in_workbook, ) from pyxform.validators.pyxform.sheet_misspellings import find_sheet_misspellings from pyxform.validators.pyxform.translations_checks import SheetTranslations -from pyxform.xls2json_backends import ( - RE_WHITESPACE, - DefinitionData, - get_xlsform, -) +from pyxform.xls2json_backends import DefinitionData, get_xlsform -SMART_QUOTES = {"\u2018": "'", "\u2019": "'", "\u201c": '"', "\u201d": '"'} -RE_SMART_QUOTES = re.compile(r"|".join(re.escape(old) for old in SMART_QUOTES)) RE_BEGIN_CONTROL = re.compile( r"^(?Pbegin)(\s|_)(?P(" + "|".join(aliases.control) @@ -97,33 +88,6 @@ def dealias_types(dict_array): return dict_array -def clean_text_values( - sheet_name: str, - data: Sequence[dict], - strip_whitespace: bool = False, - add_row_number: bool = False, -) -> Sequence[dict]: - """ - Go though the dict array and strips all text values. - Also replaces multiple spaces with single spaces. - """ - for row_number, row in enumerate(data, start=2): - for key, value in row.items(): - if isinstance(value, str) and value: - # Remove extraneous whitespace characters. - if strip_whitespace: - value = RE_WHITESPACE.sub(" ", maybe_strip(value)) - # Replace "smart" quotes with regular quotes. - row[key] = RE_SMART_QUOTES.sub(lambda m: SMART_QUOTES[m.group(0)], value) - # Check cross reference syntax. - validate_pyxform_reference_syntax( - value=value, sheet_name=sheet_name, row_number=row_number, column=key - ) - if add_row_number: - row["__row"] = row_number - return data - - def group_dictionaries_by_key(list_of_dicts, key, remove_key=True): """ Takes a list of dictionaries and returns a @@ -356,9 +320,7 @@ def workbook_to_json( header_aliases=aliases.settings_header, header_columns=set(Survey.get_slot_names()), ) - settings = clean_text_values( - sheet_name=constants.SETTINGS, data=[settings_sheet.data[0]] - )[0] + settings = settings_sheet.data[0] else: similar = find_sheet_misspellings(key=constants.SETTINGS, keys=sheet_names) if similar is not None: @@ -407,9 +369,6 @@ def workbook_to_json( # ########## External Choices sheet ########## external_choices = workbook_dict.external_choices if external_choices: - external_choices = clean_text_values( - sheet_name=constants.EXTERNAL_CHOICES, data=external_choices - ) external_choices = dealias_and_group_headers( sheet_name=constants.EXTERNAL_CHOICES, sheet_data=external_choices, @@ -426,12 +385,6 @@ def workbook_to_json( choices_sheet = workbook_dict.choices choices = {} if choices_sheet: - if clean_text_values_enabled: - choices_sheet = clean_text_values( - sheet_name=constants.CHOICES, - data=choices_sheet, - add_row_number=True, - ) choices_sheet = dealias_and_group_headers( sheet_name=constants.CHOICES, sheet_data=choices_sheet, @@ -440,6 +393,7 @@ def workbook_to_json( header_columns=option_fields, headers_required={constants.NAME}, default_language=default_language, + add_row_number=True, ) choices = group_dictionaries_by_key( list_of_dicts=choices_sheet.data, key=constants.LIST_NAME_S @@ -462,12 +416,9 @@ def workbook_to_json( # ########## Entities sheet ########### entity_declaration = None if workbook_dict.entities: - entities_sheet = clean_text_values( - sheet_name=constants.ENTITIES, data=workbook_dict.entities - ) entities_sheet = dealias_and_group_headers( sheet_name=constants.ENTITIES, - sheet_data=entities_sheet, + sheet_data=workbook_dict.entities, sheet_header=workbook_dict.entities_header, header_aliases=aliases.entities_header, header_columns={i.value for i in constants.EntityColumns.value_list()}, @@ -479,22 +430,18 @@ def workbook_to_json( warnings.append(similar + constants._MSG_SUPPRESS_SPELLING) # ########## Survey sheet ########### - survey_sheet = workbook_dict.survey # Process the headers: - if clean_text_values_enabled: - survey_sheet = clean_text_values( - sheet_name=constants.SURVEY, data=workbook_dict.survey, strip_whitespace=True - ) from pyxform.question import MultipleChoiceQuestion survey_sheet = dealias_and_group_headers( sheet_name=constants.SURVEY, - sheet_data=survey_sheet, + sheet_data=workbook_dict.survey, sheet_header=workbook_dict.survey_header, header_aliases=aliases.survey_header, header_columns=set(MultipleChoiceQuestion.get_slot_names()), headers_required={constants.TYPE}, default_language=default_language, + strip_whitespace=clean_text_values_enabled, ) survey_sheet.data = dealias_types(dict_array=survey_sheet.data) @@ -525,8 +472,6 @@ def workbook_to_json( list_of_dicts=osm_sheet.data, key=constants.LIST_NAME_S ) - # Clear references to original data for garbage collection. - del workbook_dict # ################################# # Parse the survey sheet while generating a survey in our json format: @@ -549,6 +494,7 @@ def workbook_to_json( meta_children = [] # To check that questions with triggers refer to other questions that exist. question_names = set() + element_names = Counter() trigger_references: list[tuple[str, int]] = [] repeat_names = set() @@ -850,6 +796,7 @@ def workbook_to_json( raise PyXFormError( f"{ROW_FORMAT_STRING % row_number} Invalid question name '{question_name}'. Names {XML_IDENTIFIER_ERROR_MESSAGE}" ) + element_names.update((question_name,)) unique_names.validate_question_group_repeat_name( row_number=row_number, @@ -1500,6 +1447,13 @@ def workbook_to_json( survey_children_array = stack[0]["parent_children"] survey_children_array.append(meta_element) + validate_pyxform_references_in_workbook( + workbook_dict=workbook_dict, + survey_headers=survey_sheet.headers, + choices_headers=choices_headers, + element_names=element_names, + ) + # print_pyobj_to_json(json_dict) return json_dict diff --git a/tests/entities/test_update_survey.py b/tests/entities/test_update_survey.py index 9a6b6b29..bcfe62b1 100644 --- a/tests/entities/test_update_survey.py +++ b/tests/entities/test_update_survey.py @@ -1,4 +1,5 @@ from pyxform import constants as co +from pyxform.errors import ErrorCode from tests.pyxform_test_case import PyxformTestCase from tests.xpath_helpers.entities import xpe @@ -174,3 +175,24 @@ def test_save_to_with_entity_id__puts_save_tos_on_bind(self): xpe.model_bind_question_saveto("/a", "foo"), ], ) + + def test_reference_name_not_found__error(self): + """Should raise an error if a referenced name is not in the survey sheet.""" + md = """ + | survey | + | | type | name | label | + | | text | q1 | Q1 | + + | entities | + | | list_name | entity_id | + | | e1 | ${q1x} | + """ + self.assertPyxformXform( + md=md, + errored=True, + error__contains=[ + ErrorCode.PYREF_003.value.format( + sheet="entities", column="entity_id", row=2, q="q1x" + ) + ], + ) diff --git a/tests/test_choices_sheet.py b/tests/test_choices_sheet.py index 38f428e2..7fa09ef0 100644 --- a/tests/test_choices_sheet.py +++ b/tests/test_choices_sheet.py @@ -1,3 +1,4 @@ +from pyxform.errors import ErrorCode from pyxform.validators.pyxform import choices as vc from tests.pyxform_test_case import PyxformTestCase @@ -5,7 +6,7 @@ from tests.xpath_helpers.questions import xpq -class ChoicesSheetTest(PyxformTestCase): +class TestChoicesSheet(PyxformTestCase): def test_numeric_choice_names__for_static_selects__allowed(self): """ Test numeric choice names for static selects. @@ -289,3 +290,266 @@ def test_choice_list_without_duplicates_is_successful(self): xpq.body_select1_itemset("S1"), ], ) + + def test_label_from_reference(self): + """Should find the label is an output node using the reference.""" + md = """ + | survey | + | | type | name | label | + | | select_one c1 | q1 | Q1 | + + | choices | + | | list_name | name | label | + | | c1 | n1 | ${q1} | + """ + self.assertPyxformXform( + md=md, + xml__xpath_match=[ + xpc.model_instance_choices_itext("c1", ("n1",)), + """ + /h:html/h:head/x:model/x:itext/x:translation[@lang='default'] + /x:text[@id='c1-0']/x:value[ + not(@form) + and normalize-space(./text())='' + and ./x:output[@value=' /test_name/q1 '] + ] + """, + ], + ) + + def test_label_from_reference__translated(self): + """Should find the label is an output node using the reference.""" + md = """ + | survey | + | | type | name | label | + | | select_one c1 | q1 | Q1 | + + | choices | + | | list_name | name | label::English (en) | + | | c1 | n1 | ${q1} | + """ + self.assertPyxformXform( + md=md, + xml__xpath_match=[ + xpc.model_instance_choices_itext("c1", ("n1",)), + """ + /h:html/h:head/x:model/x:itext/x:translation[@lang='English (en)'] + /x:text[@id='c1-0']/x:value[ + not(@form) + and normalize-space(./text())='' + and ./x:output[@value=' /test_name/q1 '] + ] + """, + ], + ) + + def test_label_from_reference__name_not_found__error(self): + """Should raise an error if the referenced name is not in the survey sheet.""" + md = """ + | survey | + | | type | name | label | + | | select_one c1 | q1 | Q1 | + + | choices | + | | list_name | name | label | + | | c1 | n1 | ${q1x} | + """ + self.assertPyxformXform( + md=md, + errored=True, + error__contains=[ + ErrorCode.PYREF_003.value.format( + sheet="choices", column="label", row=2, q="q1x" + ) + ], + ) + + def test_label_from_reference__name_not_found__translated__error(self): + """Should raise an error if the referenced name is not in the survey sheet.""" + md = """ + | survey | + | | type | name | label | + | | select_one c1 | q1 | Q1 | + + | choices | + | | list_name | name | label::English (en) | + | | c1 | n1 | ${q1x} | + """ + self.assertPyxformXform( + md=md, + errored=True, + error__contains=[ + ErrorCode.PYREF_003.value.format( + sheet="choices", column="label::English (en)", row=2, q="q1x" + ) + ], + ) + + def test_media_from_reference(self): + """Should find the media is an output node using the reference.""" + md = """ + | survey | + | | type | name | label | + | | select_one c1 | q1 | Q1 | + + | choices | + | | list_name | name | audio | + | | c1 | n1 | ${q1} | + """ + self.assertPyxformXform( + md=md, + xml__xpath_match=[ + xpc.model_instance_choices_itext("c1", ("n1",)), + # e.g. ' jr://audio/ ' + """ + /h:html/h:head/x:model/x:itext/x:translation[@lang='default'] + /x:text[@id='c1-0']/x:value[ + @form='audio' + and normalize-space(./text())='jr://audio/' + and ./x:output[@value=' /test_name/q1 '] + ] + """, + ], + ) + + def test_media_from_reference__translated(self): + """Should find the media is an output node using the reference.""" + md = """ + | survey | + | | type | name | label | + | | select_one c1 | q1 | Q1 | + + | choices | + | | list_name | name | audio::English (en) | + | | c1 | n1 | ${q1} | + """ + self.assertPyxformXform( + md=md, + xml__xpath_match=[ + xpc.model_instance_choices_itext("c1", ("n1",)), + """ + /h:html/h:head/x:model/x:itext/x:translation[@lang='English (en)'] + /x:text[@id='c1-0']/x:value[ + @form='audio' + and normalize-space(./text())='jr://audio/' + and ./x:output[@value=' /test_name/q1 '] + ] + """, + ], + ) + + def test_media_from_reference__name_not_found__error(self): + """Should raise an error if the referenced name is not in the survey sheet.""" + md = """ + | survey | + | | type | name | label | + | | select_one c1 | q1 | Q1 | + + | choices | + | | list_name | name | audio | + | | c1 | n1 | ${q1x} | + """ + self.assertPyxformXform( + md=md, + errored=True, + error__contains=[ + ErrorCode.PYREF_003.value.format( + sheet="choices", column="audio", row=2, q="q1x" + ) + ], + ) + + def test_media_from_reference__name_not_found__translated__error(self): + """Should raise an error if the referenced name is not in the survey sheet.""" + md = """ + | survey | + | | type | name | label | + | | select_one c1 | q1 | Q1 | + + | choices | + | | list_name | name | audio::English (en) | + | | c1 | n1 | ${q1x} | + """ + self.assertPyxformXform( + md=md, + errored=True, + error__contains=[ + ErrorCode.PYREF_003.value.format( + sheet="choices", column="audio::English (en)", row=2, q="q1x" + ) + ], + ) + + def test_reference_in_extra_columns__not_resolved(self): + """Should find that references in extra choices columns are not resolved.""" + md = """ + | survey | + | | type | name | label | + | | select_one c1 | q1 | Q1 | + + | choices | + | | list_name | name | label | extra | + | | c1 | n1 | N1 | ${q1} | + """ + self.assertPyxformXform( + md=md, + xml__xpath_match=[ + """ + /h:html/h:head/x:model/x:instance[@id='c1']/x:root/x:item[ + ./x:name/text()='n1' + and ./x:label/text()='N1' + and ./x:extra/text()='${q1}' + ] + """, + ], + ) + + def test_reference_in_extra_columns__not_validated(self): + """Should find that references in extra choices columns are not validated.""" + md = """ + | survey | + | | type | name | label | + | | select_one c1 | q1 | Q1 | + | | audio | q2 | Q2 | + + | choices | + | | list_name | name | label | unknown | bad_syntax | + | | c1 | n1 | N1 | ${q1x} | ${} | + """ + self.assertPyxformXform( + md=md, + xml__xpath_match=[ + """ + /h:html/h:head/x:model/x:instance[@id='c1']/x:root/x:item[ + ./x:name/text()='n1' + and ./x:label/text()='N1' + and ./x:unknown/text()='${q1x}' + and ./x:bad_syntax/text()='${}' + ] + """ + ], + ) + + def test_reference_in_extra_columns__between_columns_of_interest(self): + """Should find that references validation works if the extra columns are interspersed.""" + md = """ + | survey | + | | type | name | label | + | | select_one c1 | q1 | Q1 | + + | choices | + | | list_name | extra | name | label | + | | c1 | ${q1} | n1 | N1 | + """ + self.assertPyxformXform( + md=md, + xml__xpath_match=[ + """ + /h:html/h:head/x:model/x:instance[@id='c1']/x:root/x:item[ + ./x:name/text()='n1' + and ./x:label/text()='N1' + and ./x:extra/text()='${q1}' + ] + """, + ], + ) diff --git a/tests/test_fields.py b/tests/test_fields.py index b4cb6b5a..96d995a0 100644 --- a/tests/test_fields.py +++ b/tests/test_fields.py @@ -2,6 +2,7 @@ Test duplicate survey question field name. """ +from pyxform.errors import ErrorCode from pyxform.validators.pyxform import unique_names from tests.pyxform_test_case import PyxformTestCase @@ -460,3 +461,165 @@ def test_names__question_same_as_repeat_in_same_context_in_repeat__case_insensit self.assertPyxformXform( md=md, warnings__contains=[unique_names.NAMES002.format(row=4, value="Q1")] ) + + def test_reference_name_not_found__target_after_source__error(self): + """Should raise an error if the referenced name is not in the survey sheet.""" + md = """ + | survey | + | | type | name | label | + | | text | q1 | ${q2x} | + | | text | q2 | Q2 | + """ + self.assertPyxformXform( + md=md, + errored=True, + error__contains=[ + ErrorCode.PYREF_003.value.format( + sheet="survey", column="label", row=2, q="q2x" + ) + ], + ) + + def test_reference_name_not_found__target_before_source__error(self): + """Should raise an error if the referenced name is not in the survey sheet.""" + md = """ + | survey | + | | type | name | label | + | | text | q1 | Q1 | + | | text | q2 | ${q1x} | + """ + self.assertPyxformXform( + md=md, + errored=True, + error__contains=[ + ErrorCode.PYREF_003.value.format( + sheet="survey", column="label", row=3, q="q1x" + ) + ], + ) + + def test_names__question_same_as_question_in_different_group_context__with_reference__error( + self, + ): + """Should find that a referenced name needs to be unique in all contexts.""" + md = """ + | survey | + | | type | name | label | + | | text | q1 | Q1 | + | | begin group | g1 | G1 | + | | text | q1 | ${q1} | + | | end group | | | + """ + self.assertPyxformXform( + md=md, + errored=True, + error__contains=[ + ErrorCode.PYREF_004.value.format( + sheet="survey", column="label", row=4, q="q1" + ), + ], + ) + + def test_names__question_same_as_group_in_different_group_context__with_reference__error( + self, + ): + """Should find that a referenced name needs to be unique in all contexts.""" + md = """ + | survey | + | | type | name | label | + | | text | q1 | Q1 | + | | begin group | g1 | G1 | + | | begin group | q1 | G1 | + | | text | q2 | ${q1} | + | | end group | | | + | | end group | | | + """ + self.assertPyxformXform( + md=md, + errored=True, + error__contains=[ + ErrorCode.PYREF_004.value.format( + sheet="survey", column="label", row=5, q="q1" + ), + ], + ) + + def test_reference_in_ignored_columns__not_validated__type__error(self): + """Should find that references in ignored columns are not resolved.""" + md = """ + | survey | + | | type | name | label | + | | text | q1 | Q1 | + | | ${q1x} | q2 | Q2 | + """ + self.assertPyxformXform( + md=md, + errored=True, + error__contains=["Unknown question type '${q1x}'."], + ) + + def test_reference_in_ignored_columns__not_validated__name__error(self): + """Should find that references in ignored columns are not resolved.""" + md = """ + | survey | + | | type | name | label | + | | text | q1 | Q1 | + | | text | ${q1x} | Q2 | + """ + self.assertPyxformXform( + md=md, + errored=True, + error__contains=["[row : 3] Invalid question name '${q1x}'."], + ) + + def test_reference_in_ignored_columns__not_validated__name_alias__error(self): + """Should find that references in ignored columns (using an alias) are not resolved.""" + # per aliases.py, tag -> name + md = """ + | survey | + | | type | tag | label | + | | text | q1 | Q1 | + | | text | ${q1x} | Q2 | + """ + self.assertPyxformXform( + md=md, + errored=True, + error__contains=["[row : 3] Invalid question name '${q1x}'."], + ) + + def test_reference_in_aliased_column(self): + """Should find that references in a column using an alias are resolved.""" + # per aliases.py, caption -> label + md = """ + | survey | + | | type | name | caption | + | | text | q1 | Q1 | + | | text | q2 | ${q1x} | + """ + self.assertPyxformXform( + md=md, + errored=True, + error__contains=[ + ErrorCode.PYREF_003.value.format( + sheet="survey", column="caption", row=3, q="q1x" + ), + ], + ) + + def test_reference_in_aliased_column__translated(self): + """Should find that references in a translated column using an alias are resolved.""" + md = """ + | survey | + | | type | name | caption::English (en) | + | | text | q1 | Q1 | + | | text | q2 | ${q1x} | + """ + self.assertPyxformXform( + md=md, + errored=True, + error__contains=[ + ErrorCode.PYREF_003.value.format( + sheet="survey", column="caption::English (en)", row=3, q="q1x" + ), + ], + ) diff --git a/tests/test_last_saved.py b/tests/test_last_saved.py index b41186e2..5e0f6edb 100644 --- a/tests/test_last_saved.py +++ b/tests/test_last_saved.py @@ -2,6 +2,8 @@ The last-saved virtual instance can be queried to get values from the last saved instance of the form being authored. """ +from pyxform.errors import ErrorCode + from tests.pyxform_test_case import PyxformTestCase @@ -192,7 +194,9 @@ def test_last_saved_errors_when_field_does_not_exist(self): """, errored=True, error__contains=[ - "There has been a problem trying to replace ${last-saved#foo} with the XPath to the survey element named 'foo'. There is no survey element named 'foo'." + ErrorCode.PYREF_003.value.format( + sheet="survey", column="calculation", row=2, q="foo" + ) ], ) diff --git a/tests/test_settings.py b/tests/test_settings.py index a0884890..d7b2ed28 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -1,3 +1,5 @@ +from pyxform.errors import ErrorCode + from tests.pyxform_test_case import PyxformTestCase from tests.xpath_helpers.choices import xpc from tests.xpath_helpers.questions import xpq @@ -99,6 +101,51 @@ def test_clean_text_values__no(self): ], ) + def test_instance_name_from_reference(self): + """Should find a binding to set the instance name from the reference.""" + md = """ + | settings | + | | instance_name | + | | ${q1} | + + | survey | + | | type | name | label | + | | text | q1 | hello | + """ + self.assertPyxformXform( + md=md, + xml__xpath_match=[ + """ + /h:html/h:head/x:model/x:bind[ + @calculate=' /test_name/q1 ' + and @nodeset='/test_name/meta/instanceName' + and @type='string' + ] + """ + ], + ) + + def test_instance_name_from_reference__name_not_found__error(self): + """Should raise an error if the referenced name is not in the survey sheet.""" + md = """ + | settings | + | | instance_name | + | | ${q2} | + + | survey | + | | type | name | label | + | | text | q1 | hello | + """ + self.assertPyxformXform( + md=md, + errored=True, + error__contains=[ + ErrorCode.PYREF_003.value.format( + sheet="settings", column="instance_name", row=2, q="q2" + ) + ], + ) + class TestNamespaces(PyxformTestCase): """ diff --git a/tests/test_sheet_columns.py b/tests/test_sheet_columns.py index 98023853..5711b73d 100644 --- a/tests/test_sheet_columns.py +++ b/tests/test_sheet_columns.py @@ -734,6 +734,7 @@ def test_process_row__bad_header_info__unit(self): row={"a": "b", "c": "d", "e": "f"}, header_key={"a": ("a",), "c": ("b", "z")}, default_language=constants.DEFAULT_LANGUAGE_VALUE, + row_number=2, ) self.assertEqual( INVALID_HEADER.format(sheet_name="survey", header="e"), diff --git a/tests/validators/pyxform/test_pyxform_reference.py b/tests/validators/pyxform/test_pyxform_reference.py index 461e14de..2a17e059 100644 --- a/tests/validators/pyxform/test_pyxform_reference.py +++ b/tests/validators/pyxform/test_pyxform_reference.py @@ -1,3 +1,4 @@ +from collections import Counter from itertools import chain, product from pyxform.errors import ErrorCode, PyXFormError @@ -5,6 +6,7 @@ from tests.pyxform_test_case import PyxformTestCase +ELEMENT_NAMES = Counter(("a", "b", "abc123")) expression_contexts = [ ("{}", "Single reference"), ("This: {}", "Single reference with prefix"), @@ -38,7 +40,12 @@ def test_single_reference__ok(self): for token, tok_desc in ok_tokens: with self.subTest(c=context, ctx=ctx_desc, t=token, tok=tok_desc): case = context.format(token) - pr.validate_pyxform_reference_syntax(case, "test", 1, "test") + pr.validate_pyxform_reference_syntax( + sheet_name="test", + sheet_data=({"label": case},), + element_names=ELEMENT_NAMES, + limit_to_columns={"label"}, + ) def test_single_reference__error(self): """Should fail validation when the reference is malformed and used once.""" @@ -49,10 +56,15 @@ def test_single_reference__error(self): self.assertRaises(PyXFormError) as err, ): case = context.format(token) - pr.validate_pyxform_reference_syntax(case, "test", 1, "test") + pr.validate_pyxform_reference_syntax( + sheet_name="test", + sheet_data=({"label": case},), + element_names=ELEMENT_NAMES, + limit_to_columns={"label"}, + ) self.assertEqual( str(err.exception), - ErrorCode.PYREF_001.value.format(sheet="test", column="test", row=1), + ErrorCode.PYREF_001.value.format(sheet="test", column="label", row=2), msg=case, ) @@ -72,7 +84,12 @@ def test_multiple_reference__ok(self): tok_desc=(tok_desc1, tok_desc2), ): case = context.format(token1, token2) - pr.validate_pyxform_reference_syntax(case, "test", 1, "test") + pr.validate_pyxform_reference_syntax( + sheet_name="test", + sheet_data=({"label": case},), + element_names=ELEMENT_NAMES, + limit_to_columns={"label"}, + ) def test_multiple_references__error(self): """Should fail validation when one of multiple (2x) references is malformed.""" @@ -95,9 +112,14 @@ def test_multiple_references__error(self): self.assertRaises(PyXFormError) as err, ): case = context.format(token1, token2) - pr.validate_pyxform_reference_syntax(case, "test", 1, "test") + pr.validate_pyxform_reference_syntax( + sheet_name="test", + sheet_data=({"label": case},), + element_names=ELEMENT_NAMES, + limit_to_columns={"label"}, + ) self.assertEqual( str(err.exception), - ErrorCode.PYREF_001.value.format(sheet="test", column="test", row=1), + ErrorCode.PYREF_001.value.format(sheet="test", column="label", row=2), msg=case, ) From 271e6ffbec981ca37bc89cfda8c97f46e565ba15 Mon Sep 17 00:00:00 2001 From: lindsay stevens Date: Tue, 9 Dec 2025 02:36:57 +1100 Subject: [PATCH 5/8] chg: simplify node() func - move "toParseString" into func signature instead of iterating through kwargs to find it for special handling - similarly set "tag" via the first named arg rather than slicing args - ideally the same sort of thing would be done for the "unicode_args" by adding a "text: str" kwarg, but that would require a larger refactor since the text arg isn't necessarily always the second arg. --- pyxform/utils.py | 47 +++++++++++++++++++---------------------------- 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/pyxform/utils.py b/pyxform/utils.py index b211b08d..e7275c89 100644 --- a/pyxform/utils.py +++ b/pyxform/utils.py @@ -96,44 +96,35 @@ def writexml(self, writer, indent="", addindent="", newl=""): writer.write(data) -def node(*args, **kwargs) -> DetachableElement: +def node(tag: str, *args, toParseString: bool = False, **kwargs) -> DetachableElement: """ - args[0] -- a XML tag - args[1:] -- an array of children to append to the newly created node - or if a unicode arg is supplied it will be used to make a text node - kwargs -- attributes - returns a xml.dom.minidom.Element + Create an Element, with attached child elements (args) and attributes (kwargs). + + :param tag: The Element XML tag name. + :param toParseString: If True, parse the first text arg as XML and add it to the tag. """ - blocked_attributes = {"tag"} - tag = args[0] if len(args) > 0 else kwargs["tag"] - args = args[1:] result = DetachableElement(tag) unicode_args = tuple(u for u in args if isinstance(u, str)) if len(unicode_args) > 1: raise PyXFormError("""Invalid value for `unicode_args`.""") - parsed_string = False + elif len(unicode_args) == 1: + if toParseString: + # Add this header string so parseString can be used? + s = f"""<{tag}>{unicode_args[0]}""" + parsed_node = parseString(s.encode("utf-8")).documentElement + # Move node's children to the result Element + # discarding node's root + for child in parsed_node.childNodes: + result.appendChild(child.cloneNode(deep=False)) + else: + text_node = PatchedText() + text_node.data = unicode_args[0] + result.appendChild(text_node) # Convert the kwargs xml attribute dictionary to a xml.dom.minidom.Element. for k, v in kwargs.items(): - if k in blocked_attributes: - continue - if k == "toParseString": - if v is True and len(unicode_args) == 1: - parsed_string = True - # Add this header string so parseString can be used? - s = f"""<{tag}>{unicode_args[0]}""" - parsed_node = parseString(s.encode("utf-8")).documentElement - # Move node's children to the result Element - # discarding node's root - for child in parsed_node.childNodes: - result.appendChild(child.cloneNode(deep=False)) - else: - result.setAttribute(k, v) + result.setAttribute(k, v) - if len(unicode_args) == 1 and not parsed_string: - text_node = PatchedText() - text_node.data = unicode_args[0] - result.appendChild(text_node) for n in args: if isinstance(n, int | float | bytes): text_node = PatchedText() From 8b41399ce11b05fd13070f92d0fcc1347f31cdaf Mon Sep 17 00:00:00 2001 From: lindsay stevens Date: Wed, 10 Dec 2025 22:42:18 +1100 Subject: [PATCH 6/8] add: tests for parse_expression based on dynamic_default test cases --- tests/fixtures/lexer_cases.py | 81 +++++++ tests/parsing/test_expression.py | 370 ++++++++++++++++++++++++++++++- 2 files changed, 446 insertions(+), 5 deletions(-) create mode 100644 tests/fixtures/lexer_cases.py diff --git a/tests/fixtures/lexer_cases.py b/tests/fixtures/lexer_cases.py new file mode 100644 index 00000000..19ee5aa0 --- /dev/null +++ b/tests/fixtures/lexer_cases.py @@ -0,0 +1,81 @@ +from enum import Enum + + +class LexerCases(Enum): + TEXT01 = ("Literal with just alpha characters.", "foo") + TEXT02 = ("Literal with numeric characters.", "123") + TEXT03 = ("Literal with alphanumeric characters.", "bar123") + TEXT04 = ( + "Literal text containing URI; https://github.com/XLSForm/pyxform/issues/533", + "https://my-site.com", + ) + TEXT05 = ("Literal text containing brackets.", "(https://mysite.com)") + TEXT06 = ("Literal text containing URI.", "go to https://mysite.com") + TEXT07 = ( + "Literal text containing various non-operator symbols.", + "Repeat after me: '~!@#$%^&()_", + ) + TEXT08 = ("Literal text containing various non-operator symbols.", "not_func$") + TEXT09 = ("Names that look like a math expression.", "f-g") + TEXT10 = ("Names that look like a math expression.", "f-4") + TEXT11 = ("Name that looks like a math expression, in a node ref.", "./f-4") + + DATETIME01 = ("Literal date.", "2022-03-14") + DATETIME02 = ("Literal date, BCE.", "-2022-03-14") + DATETIME03 = ("Literal time.", "01:02:55") + DATETIME04 = ("Literal time, UTC.", "01:02:55Z") + DATETIME05 = ("Literal time, UTC + 0.", "01:02:55+00:00") + DATETIME06 = ("Literal time, UTC + 10.", "01:02:55+10:00") + DATETIME07 = ("Literal time, UTC - 7.", "01:02:55-07:00") + DATETIME08 = ("Literal datetime.", "2022-03-14T01:02:55") + DATETIME09 = ("Literal datetime, UTC.", "2022-03-14T01:02:55Z") + DATETIME10 = ("Literal datetime, UTC + 0.", "2022-03-14T01:02:55+00:00") + DATETIME11 = ("Literal datetime, UTC + 10.", "2022-03-14T01:02:55+10:00") + DATETIME12 = ("Literal datetime, UTC - 7.", "2022-03-14T01:02:55-07:00") + + GEO01 = ("Literal geopoint.", "32.7377112 -117.1288399 14 5.01") + GEO02 = ( + "Literal geotrace.", + "32.7377112 -117.1288399 14 5.01;32.7897897 -117.9876543 14 5.01", + ) + GEO03 = ( + "Literal geoshape.", + "32.7377112 -117.1288399 14 5.01;32.7897897 -117.9876543 14 5.01;32.1231231 -117.1145877 14 5.01", + ) + + DYNAMIC01 = ("Function call with no args.", "random()") + DYNAMIC02 = ("Function with mixture of quotes.", """ends-with('mystr', "str")""") + DYNAMIC03 = ("Function with node paths.", "ends-with(../t2, ./t4)") + DYNAMIC04 = ( + "Namespaced function. Although jr:itext probably does nothing?", + "jr:itext('/test/ref_text:label')", + ) + DYNAMIC05 = ( + "Compound expression with functions, operators, numeric/string literals.", + "if(../t2 = 'test', 1, 2) + 15 - int(1.2)", + ) + DYNAMIC06 = ( + "Compound expression with a literal first.", + "1 + decimal-date-time(now())", + ) + DYNAMIC07 = ( + "Nested function calls.", + """concat(if(../t1 = "this", 'go', "to"), "https://mysite.com")""", + ) + DYNAMIC08 = ("Two constants in a math expression.", "7 - 4") + DYNAMIC09 = ("Two constants in a math expression (mod).", "3 mod 3") + DYNAMIC10 = ("Two constants in a math expression (div).", "5 div 5") + DYNAMIC11 = ("3 or more constants in a math expression.", "2 + 3 * 4") + DYNAMIC12 = ("3 or more constants in a math expression.", "5 div 5 - 5") + DYNAMIC13 = ("Two constants, with a function call.", "random() + 2 * 5") + DYNAMIC14 = ("Node path with operator and constant.", "./f - 4") + DYNAMIC15 = ("Two node paths with operator.", "../t2 - ./t4") + DYNAMIC16 = ("Complex math expression.", "1 + 2 - 3 * 4 div 5 mod 6") + DYNAMIC17 = ("Function with date type result.", "concat('2022-03', '-14')") + DYNAMIC18 = ("Pyxform reference.", "${ref_text}") + DYNAMIC19 = ("Pyxform reference.", "${ref_int}") + DYNAMIC20 = ("Pyxform reference, with last-saved.", """${last-saved#ref_text}""") + DYNAMIC21 = ( + "Pyxform reference, with last-saved, inside a function.", + "if(${last-saved#ref_int} = '', 0, ${last-saved#ref_int} + 1)", + ) diff --git a/tests/parsing/test_expression.py b/tests/parsing/test_expression.py index ca8dc4c9..af8e5098 100644 --- a/tests/parsing/test_expression.py +++ b/tests/parsing/test_expression.py @@ -1,8 +1,11 @@ -from pyxform.parsing.expression import is_xml_tag +from enum import Enum +from pyxform.parsing.expression import is_xml_tag, parse_expression + +from tests.fixtures.lexer_cases import LexerCases from tests.pyxform_test_case import PyxformTestCase -positive = [ +tag_positive = [ ("A", "Single uppercase letter"), ("ab", "Lowercase letters"), ("_u", "Leading underscore"), @@ -20,7 +23,7 @@ ("namediv", "Contains another parser token (div)"), ] -negative = [ +tag_negative = [ ("", "Empty string"), (" ", "Space"), ("123name", "Leading digit"), @@ -38,15 +41,372 @@ ] +class ExpectedTokens(Enum): + TEXT01 = (LexerCases.TEXT01, ("NAME",)) + TEXT02 = (LexerCases.TEXT02, ("NUMBER",)) + TEXT03 = (LexerCases.TEXT03, ("NAME",)) + TEXT04 = (LexerCases.TEXT04, ("URI_SCHEME", "NAME")) + TEXT05 = (LexerCases.TEXT05, ("OPEN_PAREN", "URI_SCHEME", "NAME", "CLOSE_PAREN")) + TEXT06 = ( + LexerCases.TEXT06, + ("NAME", "WHITESPACE", "NAME", "WHITESPACE", "URI_SCHEME", "NAME"), + ) + TEXT07 = ( + LexerCases.TEXT07, + ( + "NAME", + "WHITESPACE", + "NAME", + "WHITESPACE", + "NAME", + "OTHER", + "WHITESPACE", + "OTHER", + "OTHER", + "OTHER", + "OTHER", + "OTHER", + "OTHER", + "OTHER", + "OTHER", + "OTHER", + "OPEN_PAREN", + "CLOSE_PAREN", + "NAME", + ), + ) + TEXT08 = (LexerCases.TEXT08, ("NAME", "OTHER")) + TEXT09 = (LexerCases.TEXT09, ("NAME",)) + TEXT10 = (LexerCases.TEXT10, ("NAME",)) + TEXT11 = (LexerCases.TEXT11, ("SELF_REF", "PATH_SEP", "NAME")) + + DATETIME01 = (LexerCases.DATETIME01, ("DATE",)) + DATETIME02 = (LexerCases.DATETIME02, ("DATE",)) + DATETIME03 = (LexerCases.DATETIME03, ("TIME",)) + DATETIME04 = (LexerCases.DATETIME04, ("TIME",)) + DATETIME05 = (LexerCases.DATETIME05, ("TIME",)) + DATETIME06 = (LexerCases.DATETIME06, ("TIME",)) + DATETIME07 = (LexerCases.DATETIME07, ("TIME",)) + DATETIME08 = (LexerCases.DATETIME08, ("DATETIME",)) + DATETIME09 = (LexerCases.DATETIME09, ("DATETIME",)) + DATETIME10 = (LexerCases.DATETIME10, ("DATETIME",)) + DATETIME11 = (LexerCases.DATETIME11, ("DATETIME",)) + DATETIME12 = (LexerCases.DATETIME12, ("DATETIME",)) + + GEO01 = ( + LexerCases.GEO01, + ( + "NUMBER", + "WHITESPACE", + "NUMBER", + "WHITESPACE", + "NUMBER", + "WHITESPACE", + "NUMBER", + ), + ) + GEO02 = ( + LexerCases.GEO02, + ( + "NUMBER", + "WHITESPACE", + "NUMBER", + "WHITESPACE", + "NUMBER", + "WHITESPACE", + "NUMBER", + "OTHER", + "NUMBER", + "WHITESPACE", + "NUMBER", + "WHITESPACE", + "NUMBER", + "WHITESPACE", + "NUMBER", + ), + ) + GEO03 = ( + LexerCases.GEO03, + ( + "NUMBER", + "WHITESPACE", + "NUMBER", + "WHITESPACE", + "NUMBER", + "WHITESPACE", + "NUMBER", + "OTHER", + "NUMBER", + "WHITESPACE", + "NUMBER", + "WHITESPACE", + "NUMBER", + "WHITESPACE", + "NUMBER", + "OTHER", + "NUMBER", + "WHITESPACE", + "NUMBER", + "WHITESPACE", + "NUMBER", + "WHITESPACE", + "NUMBER", + ), + ) + + DYNAMIC01 = (LexerCases.DYNAMIC01, ("FUNC_CALL", "CLOSE_PAREN")) + DYNAMIC02 = ( + LexerCases.DYNAMIC02, + ( + "FUNC_CALL", + "SYSTEM_LITERAL", + "COMMA", + "WHITESPACE", + "SYSTEM_LITERAL", + "CLOSE_PAREN", + ), + ) + DYNAMIC03 = ( + LexerCases.DYNAMIC03, + ( + "FUNC_CALL", + "PARENT_REF", + "PATH_SEP", + "NAME", + "COMMA", + "WHITESPACE", + "SELF_REF", + "PATH_SEP", + "NAME", + "CLOSE_PAREN", + ), + ) + DYNAMIC04 = (LexerCases.DYNAMIC04, ("FUNC_CALL", "SYSTEM_LITERAL", "CLOSE_PAREN")) + DYNAMIC05 = ( + LexerCases.DYNAMIC05, + ( + "FUNC_CALL", + "PARENT_REF", + "PATH_SEP", + "NAME", + "WHITESPACE", + "OPS_COMP", + "WHITESPACE", + "SYSTEM_LITERAL", + "COMMA", + "WHITESPACE", + "NUMBER", + "COMMA", + "WHITESPACE", + "NUMBER", + "CLOSE_PAREN", + "WHITESPACE", + "OPS_MATH", + "WHITESPACE", + "NUMBER", + "WHITESPACE", + "OPS_MATH", + "WHITESPACE", + "FUNC_CALL", + "NUMBER", + "CLOSE_PAREN", + ), + ) + DYNAMIC06 = ( + LexerCases.DYNAMIC06, + ( + "NUMBER", + "WHITESPACE", + "OPS_MATH", + "WHITESPACE", + "FUNC_CALL", + "FUNC_CALL", + "CLOSE_PAREN", + "CLOSE_PAREN", + ), + ) + DYNAMIC07 = ( + LexerCases.DYNAMIC07, + ( + "FUNC_CALL", + "FUNC_CALL", + "PARENT_REF", + "PATH_SEP", + "NAME", + "WHITESPACE", + "OPS_COMP", + "WHITESPACE", + "SYSTEM_LITERAL", + "COMMA", + "WHITESPACE", + "SYSTEM_LITERAL", + "COMMA", + "WHITESPACE", + "SYSTEM_LITERAL", + "CLOSE_PAREN", + "COMMA", + "WHITESPACE", + "SYSTEM_LITERAL", + "CLOSE_PAREN", + ), + ) + DYNAMIC08 = ( + LexerCases.DYNAMIC08, + ("NUMBER", "WHITESPACE", "OPS_MATH", "WHITESPACE", "NUMBER"), + ) + DYNAMIC09 = (LexerCases.DYNAMIC09, ("NUMBER", "OPS_MATH", "NUMBER")) + DYNAMIC10 = (LexerCases.DYNAMIC10, ("NUMBER", "OPS_MATH", "NUMBER")) + DYNAMIC11 = ( + LexerCases.DYNAMIC11, + ( + "NUMBER", + "WHITESPACE", + "OPS_MATH", + "WHITESPACE", + "NUMBER", + "WHITESPACE", + "OPS_MATH", + "WHITESPACE", + "NUMBER", + ), + ) + DYNAMIC12 = ( + LexerCases.DYNAMIC12, + ( + "NUMBER", + "OPS_MATH", + "NUMBER", + "WHITESPACE", + "OPS_MATH", + "WHITESPACE", + "NUMBER", + ), + ) + DYNAMIC13 = ( + LexerCases.DYNAMIC13, + ( + "FUNC_CALL", + "CLOSE_PAREN", + "WHITESPACE", + "OPS_MATH", + "WHITESPACE", + "NUMBER", + "WHITESPACE", + "OPS_MATH", + "WHITESPACE", + "NUMBER", + ), + ) + DYNAMIC14 = ( + LexerCases.DYNAMIC14, + ( + "SELF_REF", + "PATH_SEP", + "NAME", + "WHITESPACE", + "OPS_MATH", + "WHITESPACE", + "NUMBER", + ), + ) + DYNAMIC15 = ( + LexerCases.DYNAMIC15, + ( + "PARENT_REF", + "PATH_SEP", + "NAME", + "WHITESPACE", + "OPS_MATH", + "WHITESPACE", + "SELF_REF", + "PATH_SEP", + "NAME", + ), + ) + DYNAMIC16 = ( + LexerCases.DYNAMIC16, + ( + "NUMBER", + "WHITESPACE", + "OPS_MATH", + "WHITESPACE", + "NUMBER", + "WHITESPACE", + "OPS_MATH", + "WHITESPACE", + "NUMBER", + "WHITESPACE", + "OPS_MATH", + "WHITESPACE", + "NUMBER", + "OPS_MATH", + "NUMBER", + "OPS_MATH", + "NUMBER", + ), + ) + DYNAMIC17 = ( + LexerCases.DYNAMIC17, + ( + "FUNC_CALL", + "SYSTEM_LITERAL", + "COMMA", + "WHITESPACE", + "SYSTEM_LITERAL", + "CLOSE_PAREN", + ), + ) + DYNAMIC18 = (LexerCases.DYNAMIC18, ("PYXFORM_REF",)) + DYNAMIC19 = ( + LexerCases.DYNAMIC19, + ("PYXFORM_REF",), + ) + DYNAMIC20 = ( + LexerCases.DYNAMIC20, + ("PYXFORM_REF",), + ) + DYNAMIC21 = ( + LexerCases.DYNAMIC21, + ( + "FUNC_CALL", + "PYXFORM_REF", + "WHITESPACE", + "OPS_COMP", + "WHITESPACE", + "SYSTEM_LITERAL", + "COMMA", + "WHITESPACE", + "NUMBER", + "COMMA", + "WHITESPACE", + "PYXFORM_REF", + "WHITESPACE", + "OPS_MATH", + "WHITESPACE", + "NUMBER", + "CLOSE_PAREN", + ), + ) + + class TestExpression(PyxformTestCase): def test_is_xml_tag__positive(self): """Should accept positive match cases i.e. valid xml tag names.""" - for case, description in positive: + for case, description in tag_positive: with self.subTest(case=case, description=description): self.assertTrue(is_xml_tag(case)) def test_is_xml_tag__negative(self): """Should reject negative match cases i.e. invalid xml tag names.""" - for case, description in negative: + for case, description in tag_negative: with self.subTest(case=case, description=description): self.assertFalse(is_xml_tag(case)) + + def test_parse_expression(self): + """Should find expected sequence of token types for each input.""" + for lexer_case, token_types in (i.value for i in ExpectedTokens): + description, case = lexer_case.value + with self.subTest(case=case, description=description): + self.assertEqual( + token_types, tuple(t.name for t in parse_expression(text=case)[0]) + ) From 0a7bb7293853810fa992c84f0b556b2050896234 Mon Sep 17 00:00:00 2001 From: lindsay stevens Date: Wed, 10 Dec 2025 23:10:41 +1100 Subject: [PATCH 7/8] chg: replace lexer based on re.Scanner with lark - in python 3.13.10 a validation was added to re.Scanner to raise an error if a re.Scanner lexicon included a rule with more than one capturing group. This error was reverted in 3.13.11 but the relevant PRs/issues indicated that it will be permanent in python 3.14. The lexer replaced here was using multiple capturing groups but probably should have been avoiding named groups altogether since that's part of how re.Scanner identifies tokens. Also while researching the problem it seemed like the cpython developers are not supportive of anyone using re.Scanner. So this commit moves the lexing over to an equivalent `lark` grammar, which coincidentally is compiled into regex anyway. The grammar could be improved with rules to replace a) some compound terminals/tokens (e.g. FUNC_CALL) and b) external parsing functions (e.g. in instance_expression.py and pyxform_reference.py). - lark was selected since it seems to be a reasonably robust and actively supported library, with minimal dependencies (as of 1.3.1 there are none), with adequate features, and decent performance. --- pyproject.toml | 1 + pyxform/parsing/expression.py | 134 +++++++++++++------------ pyxform/parsing/instance_expression.py | 30 +++--- pyxform/utils.py | 6 +- tests/parsing/test_expression.py | 2 +- 5 files changed, 89 insertions(+), 84 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 762599a9..e6d589e7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -11,6 +11,7 @@ dependencies = [ "xlrd==2.0.1", # Read XLS files "openpyxl==3.1.5", # Read XLSX files "defusedxml==0.7.1", # Parse XML + "lark==1.3.1", # Parse custom grammars ] [project.optional-dependencies] diff --git a/pyxform/parsing/expression.py b/pyxform/parsing/expression.py index 6f0edbf2..014d4e35 100644 --- a/pyxform/parsing/expression.py +++ b/pyxform/parsing/expression.py @@ -2,6 +2,8 @@ from functools import lru_cache from typing import Any +from lark import Lark, Token + # ncname regex adapted from eulxml https://github.com/emory-libraries/eulxml/blob/2e1a9f71ffd1fd455bd8326ec82125e333b352e0/eulxml/xpath/lexrules.py # (C) 2010,2011 Emory University Libraries [Apache v2.0 License] # They in turn adapted it from https://www.w3.org/TR/REC-xml/#NT-NameStartChar @@ -20,10 +22,6 @@ ncname_regex_ns = rf"{ncname_regex}(?:\:{ncname_regex})?" ncname_regex_ns_named = rf"(?P{ncname_regex_ns})" -date_regex = r"-?\d{4}-\d{2}-\d{2}" -time_regex = r"\d{2}:\d{2}:\d{2}(\.\s+)?(((\+|\-)\d{2}:\d{2})|Z)?" -date_time_regex = date_regex + "T" + time_regex - # pyxform_ref_outer picks up possible refs, and matches unterminated refs to exclude them. pyxform_ref_outer = r"\$\{(?P[^}]+)\}|\$\{[^}]*$" pyxform_ref_inner = rf"(?Plast-saved#)?{ncname_regex_named}" @@ -32,38 +30,68 @@ ) pyxform_ref = rf"(?P\$\{{{pyxform_ref_inner}\}})" -# Rule order is significant - match priority runs top to bottom. -LEXER_RULES = { - # https://www.w3.org/TR/xmlschema-2/#dateTime - "DATETIME": date_time_regex, - "DATE": date_regex, - "TIME": time_regex, - "NUMBER": r"-?\d+\.\d*|-?\.\d+|-?\d+", - # https://www.w3.org/TR/1999/REC-xpath-19991116/#exprlex - "OPS_MATH": r"[\*\+\-]| mod | div ", - "OPS_COMP": r"\=|\!\=|\<|\>|\<=|>=", - "OPS_BOOL": r" and | or ", - "OPS_UNION": r"\|", - "OPEN_PAREN": r"\(", - "CLOSE_PAREN": r"\)", - "BRACKET": r"\[\]\{\}", - "PARENT_REF": r"\.\.", - "SELF_REF": r"\.", - "PATH_SEP": r"\/", # javarosa.xpath says "//" is an "unsupported construct". - "SYSTEM_LITERAL": r""""[^"]*"|'[^']*'""", - "COMMA": r",", - "WHITESPACE": r"\s+", - "PYXFORM_REF": pyxform_ref, - "FUNC_CALL": ncname_regex_ns_named + r"\(", - "XPATH_PRED_START": ncname_regex_ns_named + r"\[", - "XPATH_PRED_END": r"\]", - "URI_SCHEME": ncname_regex_named + r"://", - "NAME": ncname_regex_named, # Must be after rules containing ncname_regex. - "PYXFORM_REF_START": r"\$\{", - "PYXFORM_REF_END": r"\}", - "OTHER": r".+?", # Catch any other character so that parsing doesn't stop. -} - +lark_grammar = rf""" + // Parser + start: (token | WHITESPACE)* + ?token: DATETIME + | DATE + | TIME + | NUMBER + | OPS_MATH + | OPS_COMP + | OPS_BOOL + | OPS_UNION + | OPEN_PAREN + | CLOSE_PAREN + | BRACKET + | PARENT_REF + | SELF_REF + | PATH_SEP + | SYSTEM_LITERAL + | COMMA + | PYXFORM_REF + | FUNC_CALL + | XPATH_PRED_START + | XPATH_PRED_END + | URI_SCHEME + | NAME + | PYXFORM_REF_START + | PYXFORM_REF_END + | OTHER + + // Lexer + // https://www.w3.org/TR/xmlschema-2/#dateTime + DATETIME.25: DATE "T" TIME + DATE.24: /-?\d{{4}}-\d{{2}}-\d{{2}}/ + TIME.23: /\d{{2}}:\d{{2}}:\d{{2}}(\.\s+)?(((\+|\-)\d{{2}}:\d{{2}})|Z)?/ + NUMBER.22: /-?\d+\.\d*|-?\.\d+|-?\d+/ + // https://www.w3.org/TR/1999/REC-xpath-19991116/#exprlex + OPS_MATH.21: /[\*\+\-]| mod | div / + OPS_COMP.20: /\=|\!\=|\<|\>|\<=|>=/ + OPS_BOOL.19: / and | or / + OPS_UNION.18: /\|/ + OPEN_PAREN.17: /\(/ + CLOSE_PAREN.16: /\)/ + BRACKET.15: /[\[\{{\}}]/ + PARENT_REF.14: /\.\./ + SELF_REF.13: /\./\ + // # javarosa.xpath says "//" is an "unsupported construct". + PATH_SEP.12: /\// + SYSTEM_LITERAL.11: /"[^"]*"|'[^']*'/ + COMMA.10: /,/ + WHITESPACE.9: /\s+/ + PYXFORM_REF.8: /\$\{{(?:last-saved#)?{ncname_regex}\}}/ + FUNC_CALL.7: /{ncname_regex_ns}\(/ + XPATH_PRED_START.6: /{ncname_regex_ns}\[/ + XPATH_PRED_END.5: /\]/ + URI_SCHEME.4: /{ncname_regex}:\/\// + // Must be lower priority than rules containing ncname_regex. + NAME.3: /{ncname_regex_ns}/ + PYXFORM_REF_START.2: /\$\{{/ + PYXFORM_REF_END.1: /\}}/\ + // Catch any other character so that parsing doesn't stop. + OTHER.0: /.+?/\ +""" RE_NCNAME_NAMESPACED = re.compile(ncname_regex_ns_named) RE_PYXFORM_REF = re.compile(pyxform_ref) @@ -71,36 +99,13 @@ RE_PYXFORM_REF_INNER = re.compile(pyxform_ref_inner) -def get_expression_lexer() -> re.Scanner: - def get_tokenizer(name): - def tokenizer(scan, value) -> ExpLexerToken | str: - match = scan.match - return ExpLexerToken(name, value, match.start(), match.end()) - - return tokenizer - - lexicon = [(v, get_tokenizer(k)) for k, v in LEXER_RULES.items()] - # re.Scanner is undocumented but has been around since at least 2003 - # https://mail.python.org/pipermail/python-dev/2003-April/035075.html - return re.Scanner(lexicon) - - -class ExpLexerToken: - __slots__ = ("end", "name", "start", "value") - - def __init__(self, name: str, value: str, start: int, end: int) -> None: - self.name: str = name - self.value: str = value - self.start: int = start - self.end: int = end - - -# Scanner takes a few 100ms to compile so use the shared instance. -_EXPRESSION_LEXER = get_expression_lexer() +_EXPRESSION_LEXER = Lark( + lark_grammar, parser="lalr", start="start", propagate_positions=True +) @lru_cache(maxsize=128) -def parse_expression(text: str) -> tuple[list[ExpLexerToken], str]: +def parse_expression(text: str) -> tuple[Token, ...]: """ Parse an expression. @@ -109,8 +114,7 @@ def parse_expression(text: str) -> tuple[list[ExpLexerToken], str]: :param text: The expression. :return: The parsed tokens, and any remaining unparsed text. """ - tokens, remainder = _EXPRESSION_LEXER.scan(text) - return tokens, remainder + return tuple(_EXPRESSION_LEXER.lex(text)) def is_xml_tag(value: str) -> bool: diff --git a/pyxform/parsing/instance_expression.py b/pyxform/parsing/instance_expression.py index 2090c260..f7cfc861 100644 --- a/pyxform/parsing/instance_expression.py +++ b/pyxform/parsing/instance_expression.py @@ -21,7 +21,7 @@ def find_boundaries(xml_text: str) -> list[tuple[int, int]]: :param xml_text: XML text that may contain an instance expression. :return: Tokens in instance expression, and the string position boundaries. """ - tokens, _ = parse_expression(xml_text) + tokens = parse_expression(xml_text) if not tokens: return [] instance_enter = False @@ -33,43 +33,43 @@ def find_boundaries(xml_text: str) -> list[tuple[int, int]]: for t in tokens: emit = False # If an instance expression had started, note the string position boundary. - if not instance_enter and t.name == "FUNC_CALL" and t.value == "instance(": + if not instance_enter and t.type == "FUNC_CALL" and t.value == "instance(": instance_enter = True emit = True - boundaries.append(t.start) + boundaries.append(t.start_pos) # Tokens that are part of an instance expression. elif instance_enter: # Tokens that are part of the instance call. if ( - t.name == "SYSTEM_LITERAL" - and last_token.name == "FUNC_CALL" + t.type == "SYSTEM_LITERAL" + and last_token.type == "FUNC_CALL" and last_token.value == "instance(" ): emit = True - elif last_token.name == "SYSTEM_LITERAL" and t.name == "CLOSE_PAREN": + elif last_token.type == "SYSTEM_LITERAL" and t.type == "CLOSE_PAREN": emit = True - elif t.name == "PATH_SEP" and last_token.name == "CLOSE_PAREN": + elif t.type == "PATH_SEP" and last_token.type == "CLOSE_PAREN": emit = True path_enter = True # A XPath path may continue after a predicate. - elif t.name == "PATH_SEP" and last_token.name == "XPATH_PRED_END": + elif t.type == "PATH_SEP" and last_token.type == "XPATH_PRED_END": emit = True path_enter = True # Tokens that are part of a XPath path. elif path_enter: - if t.name == "WHITESPACE": + if t.type == "WHITESPACE": path_enter = False - elif t.name != "XPATH_PRED_START": + elif t.type != "XPATH_PRED_START": emit = True - elif t.name == "XPATH_PRED_START": + elif t.type == "XPATH_PRED_START": emit = True path_enter = False pred_enter = True # Tokens that are part of a XPath predicate. elif pred_enter: - if t.name != "XPATH_PRED_END": + if t.type != "XPATH_PRED_END": emit = True - elif t.name == "XPATH_PRED_END": + elif t.type == "XPATH_PRED_END": emit = True pred_enter = False # Track instance expression tokens, ignore others. @@ -78,10 +78,10 @@ def find_boundaries(xml_text: str) -> list[tuple[int, int]]: # If an instance expression had ended, note the string position boundary. elif instance_enter: instance_enter = False - boundaries.append(last_token.end) + boundaries.append(last_token.end_pos) if last_token is not None: - boundaries.append(last_token.end) + boundaries.append(last_token.end_pos) # Pair up the boundaries [1, 2, 3, 4] -> [(1, 2), (3, 4)]. bounds = iter(boundaries) diff --git a/pyxform/utils.py b/pyxform/utils.py index e7275c89..310f59d6 100644 --- a/pyxform/utils.py +++ b/pyxform/utils.py @@ -232,16 +232,16 @@ def default_is_dynamic(element_default, element_type=None): if not element_default or not isinstance(element_default, str): return False - tokens, _ = parse_expression(element_default) + tokens = parse_expression(element_default) for t in tokens: # Data types which are likely to have non-dynamic defaults containing a hyphen. if element_type in {"date", "dateTime", "geopoint", "geotrace", "geoshape"}: # Nested to avoid extra string comparisons if not a relevant data type. - if t.name == "OPS_MATH" and t.value == "-": + if t.type == "OPS_MATH" and t.value == "-": return False # A match on these lexer rules indicates a dynamic default. - if t.name in { + if t.type in { "OPS_MATH", "OPS_UNION", "XPATH_PRED", diff --git a/tests/parsing/test_expression.py b/tests/parsing/test_expression.py index af8e5098..59dc646c 100644 --- a/tests/parsing/test_expression.py +++ b/tests/parsing/test_expression.py @@ -408,5 +408,5 @@ def test_parse_expression(self): description, case = lexer_case.value with self.subTest(case=case, description=description): self.assertEqual( - token_types, tuple(t.name for t in parse_expression(text=case)[0]) + token_types, tuple(t.type for t in parse_expression(text=case)) ) From dddb5635b4e81808a97196bfe4efa4758df69eec Mon Sep 17 00:00:00 2001 From: lindsay stevens Date: Wed, 10 Dec 2025 23:12:06 +1100 Subject: [PATCH 8/8] chg: remove redundant isinstance() check - L791 does a str() conversion so question_name can never be bytes --- pyxform/xls2json.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py index 53a981f8..2bac009f 100644 --- a/pyxform/xls2json.py +++ b/pyxform/xls2json.py @@ -790,9 +790,6 @@ def workbook_to_json( ) question_name = str(row[constants.NAME]) if not is_xml_tag(question_name): - if isinstance(question_name, bytes): - question_name = question_name.decode("utf-8") - raise PyXFormError( f"{ROW_FORMAT_STRING % row_number} Invalid question name '{question_name}'. Names {XML_IDENTIFIER_ERROR_MESSAGE}" )